Complete the MVar2 interface

[vasilmkd]

Attempt at resolving #820.

The implementation aims to follow the semantics described here and here.

Please take a look at the tests and comment whether you think they are acceptable and/or sufficient.

[SystemFw]

Generally torn here. I think the implementation for modify is the best you can do in the CE2 model (short of trying to implement it inside MVar like Semaphore does), but it comes at quite a cost if you look at what continual does, and the resulting semantics are still more unsafe than Semaphore.withPermit in that a concurrent writer can still mean you deadlock. At least right now the absence of these functions prevents this.

Anyway, let's see what the others think :)

[vasilmkd]

@SystemFw I agree completely. I did put your concern in the scaladoc for each of the new methods. It is however, the exact same unsafety in terms of deadlocks that is also present in Haskell's MVar, and there too, only the documentation is in place to warn the user.

Anyways, it was fun writing this PR, I got to explore the different cancelation semantics in CE as it currently stands. I'm also fine if none of this makes it into the final code.

[rossabaker]

In a hurry tonight, so just a quick reaction for now: I'd say the deadlock situation in Haskell is somewhat less severe, because the runtime can detect threads deadlocked on MVar misuse and throw an exception. It's bad in Haskell, but you get to clean up and see what went wrong, which is better than hanging.

This MVar incubated in Monix, and Monix still has its own, so I'm curious what @alexandru thinks.

I hope that I did not lead us astray by proposing these Haskell methods without thinking through their fit in Scala, but I'm glad you had fun either way.

[vasilmkd]

However, if we do end up dropping this PR, swap in the Concurrent implementation is currently unsafe in terms of cancelation (cancelation can occur between reading and writing the MVar) which I believe is not desirable, and something should be done about that.

[vasilmkd]

Generally torn here. I think the implementation for modify is the best you can do in the CE2 model (short of trying to implement it inside MVar like Semaphore does), but it comes at quite a cost if you look at what continual does, and the resulting semantics are still more unsafe than Semaphore.withPermit in that a concurrent writer can still mean you deadlock. At least right now the absence of these functions prevents this.

Anyway, let's see what the others think :)

@SystemFw can you describe shortly what would the desirable semantics of a Concurrent modify be (w.r.t. cancelation)? When compared to Semaphore, the cleanup releaseN is still uncancelable (being executed as the release action of a Bracket), but that is unsafe for MVar.

Thus, if replacing the old value of the MVar should be cancelable in Scala (otherwise it would lead to deadlocks), why not just go with a simple imlementation similar to the current swap, where cancelation can occur anywhere.

Am I missing something? /cc @rossabaker

[djspiewak]

@SystemFw @rossabaker Any thoughts on this? I'd like to unblock things. These honestly seem like reasonable functions, and they should be safe on ce3, so it's reasonably future-proof. Losing mapK is unfortunate, though not the end of the world.

[SystemFw]

I'm still not a massive fan of them (not the implementation, which is good :) ), just on principle I think MVar is a bit of a trap, mitigated in haskell by a mechanism that we do not have.

That being said, the implementation is good (especially in ce3 when we can remove the weight of all the continuals), and other people like MVar, so we can merge

Also, apologies to @vasilmkd for taking so long to reply

[vasilmkd]

Totally fine. Please do not feel the need to merge just because the code is already written. It should only be merged if it's right to do so. You guys certainly know much more about all of this.

[djspiewak]

Totally fine. Please do not feel the need to merge just because the code is already written. It should only be merged if it's right to do so. You guys certainly know much more about all of this.

This just mostly comes down to the fact that MVar is terrible. :-) I think we're all very interested in finding ways to steer people away from MVar towards safer and easier alternatives (Ref/Deferred/etc). I do think these functions are fine in so far as MVar goes, it's just that MVar is sadness.

At any rate… your work here is excellent! Thank you!