Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that ce3 MVar is in sync with ce2 #967

Closed
djspiewak opened this issue Jul 20, 2020 · 15 comments
Closed

Ensure that ce3 MVar is in sync with ce2 #967

djspiewak opened this issue Jul 20, 2020 · 15 comments

Comments

@djspiewak
Copy link
Member

See #912 for some context.

@djspiewak djspiewak added the CE 3 label Jul 20, 2020
@vasilmkd
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

Thanks. I'll check it out.

@vasilmkd
Copy link
Member

vasilmkd commented Jul 21, 2020

@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
Copy link
Contributor

@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 #681

@vasilmkd
Copy link
Member

vasilmkd commented Jul 21, 2020

@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
Copy link
Contributor

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
Copy link
Member

So it's ok if they are uninterruptible?

@SystemFw
Copy link
Contributor

SystemFw commented Jul 21, 2020

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
Copy link
Member

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
Copy link
Contributor

SystemFw commented Jul 21, 2020

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
Copy link
Member

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

@SystemFw
Copy link
Contributor

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
Copy link
Member

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

@djspiewak
Copy link
Member Author

Not sure! Closing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants