Cats-effect: Ensure that ce3 MVar is in sync with ce2

[djspiewak]

See #912 for some context.

[vasilmkd]

I could work on this. Not sure what the state of CE3 is at the moment, but I definitely have context on the issue.

[djspiewak]

I could work on this. Not sure what the state of CE3 is at the moment, but I definitely have context on the issue.

That would be amazing! You should just be able to checkout series/3.x and go to town.

[vasilmkd]

Thanks. I'll check it out.

[vasilmkd]

@djspiewak I have a question. Essentially, the put action needs to happen whenever the take action succeeds. However, given that the only way to guarantee execution of an F is by using bracket, onCancel or onError, all of these mechanisms still force the F to be uncancelable, therefore making put uncancelable. I feel we are back to square one, in terms of unsafety. Haskell has a special mechanism to cancel the put, but I don't see a way to replicate that in CE3. Thoughts? Please use #912 for context.

[SystemFw]

@vasilmkd you need to look at uncancelable , bracket is no longer primitive. It's a bit like uninterruptibleMask, if you're familiar with that. Look at my initial proposal here https://github.com/typelevel/cats-effect/issues/681

[vasilmkd]

@SystemFw Thanks for chiming in. I have looked at it. I guess my biggest problem right now is the onException line from the Haskell implementation. How do I make sure to set the value again after cancelation, but make that put action cancelable too?

Source here: http://hackage.haskell.org/package/base-4.14.0.0/docs/src/Control.Concurrent.MVar.html#modifyMVar

[SystemFw]

In haskell interruption and errors are both represented by (different types of) exception, which is actually a source of pain. Also you can't port the haskell version directly, that's written with mask whereas we offer uninterruptible mask (by design). Basically you will have uncancelable { poll => ... } and you need to call poll(code that you want to make interruptible), in this case at least takeMVar and io a. You can use onCancel/onError/onCase to execute finalisers, and I'm actually not sure you want _those_ to be interruptible

[vasilmkd]

So it's ok if they are uninterruptible?

[SystemFw]

The put? In my opinion yes, because that's restoring the value, you don't want that to be interrupted as well, or the mvar will be left in an invalid state

[vasilmkd]

The way I understood your objections to the code that was merged in CE2, is that in case of two producers, a deadlock can occur because the put is uninterruptible. And I thought that it needed changing. Because in Haskell, the deadlock can be detected and the put canceled. I thought, we could at least help there partially, by making it cancelable.

Otherwise, I too, think that put should be uncancelable.

[SystemFw]

is that in case of two producers, a deadlock can occur because the put is uninterruptible

Well, if you think about it that's not strictly true: a deadlock can happen because of the semantics of MVar (which is partly why I don't like it, it's intrinsic) , and you might somehow mitigate the situation because of put being interruptible* due to deadlock detection. But we don't have deadlock detection, so I think it's just better to make it uncancelable (the one in onCancel). The final put is more interesting, but on balance I'd say that should be uninterruptible as well. EDIT: actually I'm not sure about the second put, but in any case the difference is an additional poll, we can discuss.

*Btw, interestingly enough deadlock detection in Haskell is not "interruption", strictly speaking, in the sense that it's not an async exception. So even if you uninterruptibleMask_ in haskell, deadlock detection will still trigger a failure. That's because the exception triggered by deadlock detection happens on the same thread that is being blocked, as a synchronous exception.

[vasilmkd]

Thanks for taking the time to explain @SystemFw. I appreciate it.

[SystemFw]

Btw the intrinsic limitations of MVar are the same in ce2 and ce3, but in ce2 the workaround is heavyweight, since continual is creating extra fibers, finalizers, Deferreds etc

[vasilmkd]

I wonder why this issue didn't close automatically. @djspiewak

[djspiewak]

Not sure! Closing now.