<div dir="ltr">It strikes me that this is winding up highly controversial.<div><br></div><div>Assuming that, well, things might not go your way in terms of getting <span style="font-family:monospace">bracket</span> changed, let's think a bit about what a more retrenched solution would look like. </div><div><br></div><div>This way we have a continuum of fixes and can try to find the right point in the continuum.<br></div><div><br></div><div><div>So let's put forth a couple more colors for the bikeshed:</div></div><div><br></div><div><b>Documentation Only</b></div><div><br></div><div><div>The issue is that users use <span style="font-family:monospace">bracket</span> assuming things will be safer than they are. At the very least we need to clearly document the fact that this isn't the case!</div></div><div><br></div><div>We could document the pattern of using <font face="monospace">uninterruptibleMask_</font> yourself in handlers for safety and include examples of where it is required.<br></div><div><br></div><div><b>Combinators</b></div><div><br></div><div>If we want to go further, we could introduce:</div><div><br></div><div><span style="font-family:monospace">uninterruptibleBracket :: IO a -> (a -> IO b) -> (a -> IO c) -> IO c</span><br></div><div><font face="monospace">uninterruptibleBracket acquire release = bracket acquire (uninterruptibleMask_ . release)</font></div><div><br></div><div><div><font face="monospace">uninterruptibleBracket_ :: IO a -> IO b -> IO c -> IO c</font></div><div><font face="monospace">uninterruptibleBracket_ acquire release = bracket_ acquire (uninterruptibleMask_ release)</font></div></div><div><font face="monospace"><br></font></div><div><font face="monospace">etc.</font></div><div><br></div><div>and then upgrade the documentation for `bracket` and the like with a big fat warning about how certain common sense examples using <font face="monospace">bracket</font> should really be using <font face="monospace">uninterruptibleBracket.</font></div><div><br></div><div><b>Merijn's Proposal</b><br></div><div><b><br></b></div><div>We could go further and switch the default behavior of bracket to that of uninterruptibleBracket above. Then we get Merijn's proposal.</div><div><br></div><div><div><b>Going Further</b><br></div></div><div><br></div><div>But if we do that we should probably consider adding an `interruptibleBracket` that matches the existing behavior with the same caveats we would want to put on `bracket`.</div><div><br></div><div>The use cases that folks have that center around weird RPC handling scenarios in the release handler seem to fit into this niche.</div><div><br></div><div>This redefines <font face="monospace">bracket</font><font face="arial, helvetica, sans-serif">, like Merijn would prefer, </font>because it is a source of very very hard to track down resource bugs, and make the few who actually want to do complex code that relies on active asynchronous exception support in the handler switch combinators. </div><div><br></div><div>It also has the benefit that the name <font face="monospace">interruptibleBracket</font> is easier to explain than <font face="monospace">uninterruptibleBracket</font>, which only made release uninterruptible.</div><div><br></div><div>The kind of code that would be affected is the kind of code that would be very visibly affected, whereas the kind of code that is broken right now is scattered across the entire ecosystem and is just subtly wrong.<br></div><div><br></div><div><b>Personal Thoughts</b></div><div><br></div><div>I started writing this proposal with a continuum in mind, thinking I'd land somewhere in the middle.</div><div><br></div><div><div>Normally, I'd be disinclined to change semantics on a function with such widespread use! I very strongly sympathize with Greg's position here.</div></div><div><br></div><div><div>However, personally, I think the "Going Further" solution above is the right solution, which is effectively Merijn's proposal with the addition of <font face="monospace">interruptibleBracket</font> and the like.</div><div><br></div></div><div><div>However, in this case the only code that really can rely upon this behavior is code that happens to know it won't kill the thread from outside until it is in the handler, but that they want to do a thing that will throw them asynchronous exceptions _within_ the handler, and well, that is a marginal enough use case that I don't have much a problem marginalizing it further by forcing its practitioners to use a more exotic combinator. If they are absolutely allergic to the new semantics you can always swap all the existing uses of <font face="monospace">bracket</font> to <font face="monospace">interruptibleBracket</font><font face="arial, helvetica, sans-serif">, and the very kind of user who would need these semantics is the kind of user who is equipped to carry out this sort of change.</font></div></div><div><font face="arial, helvetica, sans-serif"><br></font></div><div><font face="arial, helvetica, sans-serif">This effectively puts me at +1 with the caveat that I'd like to see </font><font face="monospace">interruptibleBracket</font><font face="arial, helvetica, sans-serif"> added.</font></div><div><br></div><div>-Edward</div><div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 11, 2014 at 1:09 PM, Merijn Verstraaten <span dir="ltr"><<a href="mailto:merijn@inconsistent.nl" target="_blank">merijn@inconsistent.nl</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ola!<br>
<br>
In September Eyal Lotem raised the issue of bracket's cleanup handler not being uninterruptible [1]. This is a final bikeshedding email before I submit a patch.<br>
<br>
The problem, summarised:<br>
Blocking cleanup actions can be interrupted, causing cleanup not to happen and potentially leaking resources.<br>
<br>
Main objection to making the cleanup handler uninterruptible:<br>
Could cause deadlock if the code relies on async exceptions to interrupt a blocked thread.<br>
<br>
I count only two objections in the previous thread, 1 on the grounds that "deadlocks are NOT unlikely" and 1 that is conditioned on "I don't believe this is a problem".<br>
<br>
The rest seems either +1, or at least agrees that the status quo is *worse* than the proposed solution.<br>
<br>
My counter to these objections is:<br>
1) No one has yet shown me any code that relies on the cleanup handler being interruptible<br>
<br>
2) There are plenty of examples of current code being broken, for example every single 'bracket' using file handles is broken due to handle operations using a potentially blocking MVar operation internally, potentially leaking file descriptors/handles.<br>
<br>
3) Even GHC-HQ can't use bracket correctly (see Simon's emails)<br>
<br>
Potential solution #1:<br>
Leave bracket as-is, add bracketUninterruptible with an uninterruptible cleanup handler.<br>
<br>
Potential solution #2:<br>
Change bracket to use uninterruptible cleanup handler, add bracketInterruptible for interruptible cleanups.<br>
<br>
Trade-offs:<br>
Solution 1 won't change the semantics of any existing code, however this also means that any currently broken uses of bracket will remain broken, possibly indefinitely.<br>
<br>
Solution 2 will change the semantics of bracket, which means any currently broken uses of bracket will be fixed, at the cost of creating potential deadlocks in code that relies on the interruptibility of cleanup.<br>
<br>
I will argue that solution #2 is preferable, since I have yet to see any code that uses the interruptibility of the cleanup handler. Whereas there's many broken assumption assuming the cleanup handler is not interruptible.<br>
<br>
Secondly, it is easier to detect deadlocks caused by this problem than it is to detect resource leaks which only happen in unlucky timings of async exceptions. Especially since any deadlock caused by the change can be fixed by replacing bracket with bracketInterruptible.<br>
<br>
[1] - <a href="https://www.haskell.org/pipermail/libraries/2014-September/023675.html" target="_blank">https://www.haskell.org/pipermail/libraries/2014-September/023675.html</a><br>
<br>
Cheers,<br>
Merijn<br>
_______________________________________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org">Libraries@haskell.org</a><br>
<a href="http://www.haskell.org/mailman/listinfo/libraries" target="_blank">http://www.haskell.org/mailman/listinfo/libraries</a><br>
</blockquote></div><br></div>