<div dir="ltr">I suspect the main reason someone would want an interruptible cleanup handler is because if the action blocks, it's the only way to kill the thread. I'm not sure how common this case is though.<div>
<br></div><div>IMHO I think the real problem is that the current exception implementation is far too complex to reason about. Do other languages have this interruptible/uninterruptible distinction? If so, I've yet to encounter it. I'm starting to think that throwing an exception to a thread is the wrong way to kill it, and it should use something other than the exception mechanism. Then I think we could discard the notion of interruptible actions entirely.</div>
<div><br></div><div>I guess count me as +0.5 for this proposal.</div><div><br></div><div>John L.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 10:01 PM, Eyal Lotem <span dir="ltr"><<a href="mailto:eyal.lotem@gmail.com" target="_blank">eyal.lotem@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">The problem with requiring the user to use uninterruptibleMask_ on their cleanup is how error-prone it is.<div>
<br></div><div>If we examine some uses of bracket in the GHC repo:</div><div><div><br>
</div></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><div>compiler/main/GhcMake.hs:992: let withSem sem = bracket_ (waitQSem sem) (<b>signalQSem sem</b>)</div></div><div><b>signalQSem is interruptible, this is a bug!</b></div>
<div><b><br></b></div><div>bracket_ (hSetBinaryMode h False) (<b>hSetBinaryMode h True</b>)</div><div>Is the hSetBinaryMode operation interruptible? Is this a bug? Hard to tell!</div><div><br></div><div><div>withExtendedLinkEnv new_env action</div>
</div><div><div> = gbracket (liftIO $ extendLinkEnv new_env)</div></div><div><div> (\_ -> <b>reset_old_env</b>)</div></div><div><div> (\_ -> action)</div></div><div><br></div><div><b>reset_old_env </b>uses MVars so is probably interruptible, probably a bug.</div>
<div><br></div></blockquote><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div>Lots of manual openFile/hClose duplications of withFile, which are all buggy due to <b>hClose</b> being interruptible.</div>
<div><br></div></blockquote><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><div>InteractiveUI.doLoad:</div></div><div><div> gbracket (liftIO $ do hSetBuffering stdout LineBuffering</div><div> hSetBuffering stderr LineBuffering)</div>
<div> (\_ -></div><div> liftIO $ do<b> hSetBuffering stdout NoBuffering</b></div><div><span style="font-weight:bold"> hSetBuffering stderr NoBuffering</span>) $ \_ -> do</div>
<div style="font-weight:bold"><br></div></div></blockquote><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div>hSetBuffering uses hFlush and is thus interruptible(?).</div></blockquote><div><br></div>
<div>If many uses of bracket by the GHC developers are broken, is it not a clear indication that the default behavior is too error prone?</div><div><br></div><div>If someone wants an interruptible cleanup handler (Very weird thing to want!) they can use "mask" and "onException" directly. Or variants of all the cleanup functions with an "interruptible" suffix can be exported too (bracketInterruptible, catchInterruptible, etc).</div>
<div><br></div><div>Keeping the current behavior has only one benefit: Use cases that need to have interruptible cleanups.</div><div>Does anyone have any such use case at all? I can't imagine one...</div>
</div><div class="gmail_extra"><div><div class="h5"><br><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 10:46 AM, John Lato <span dir="ltr"><<a href="mailto:jwlato@gmail.com" target="_blank">jwlato@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>Being a primop or not has nothing to do with whether an MVar operation is interruptible. What matters is whether or not the operation will block. withMVar (or readMVar in any incarnation) on an empty MVar is interruptible. If the MVar happened to be full, it's not interruptible.</div>
<div><br></div><div>I agree this is a problem. I don't think the proposed solution is perfect, but I do think it's possibly better than the status quo. Perhaps the user should be required to use uninterruptibleMask_ on the cleanup action if necessary? I've long thought that using an interruptible operation in a cleanup handler is a programming error.</div>
<div><br></div><div>John L.</div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 12:29 AM, Eyal Lotem <span dir="ltr"><<a href="mailto:eyal.lotem@gmail.com" target="_blank">eyal.lotem@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">In addition to hClose, the majority of cleanup handlers in my programs turned out to be interruptible. Moreover, whether something is interruptible is unclear. You can easily add a putStrLn to a cleanup handler and now it is accidentally interruptible.</p>
<p dir="ltr">I'd love to see examples of some code where interruptible cleanup handlers are not a bug.</p>
<p dir="ltr">Every single one in my programs that I examined was a bug.</p>
<p dir="ltr">Is withMVar also a primop? Because it's buggy in the same way as withFile currently is.</p>
<p dir="ltr">The current situation is that virtually all uses of bracket in the entire Haskell ecosystem are subtle bugs. </p><div><div>
<div class="gmail_quote">On Sep 4, 2014 3:01 AM, "Gregory Collins" <<a href="mailto:greg@gregorycollins.net" target="_blank">greg@gregorycollins.net</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Unless I'm mistaken, here the "mask" call inside bracket already makes sure you don't receive asynchronous exceptions unless you call a function that is interruptible (i.e. goes back into the runtime system). The hClose example you give doesn't fall in this category, as something inside the RTS needs to call "allowInterrupt" (or otherwise unmask exceptions) in order for async exceptions to be delivered. The "readMVar" example you give *does* have this issue (because putMVar does an implicit allowInterrupt) but in recent GHC readMVar has been redefined as a primop.<div>
<br></div><div>The danger of deadlock is *not* minimal here, doing what you suggest will transform many valid programs (i.e. if you block on a "takeMVar" in the cleanup action) into ones that have unkillable orphan threads.</div>
<div><br></div><div>G</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Sep 3, 2014 at 1:56 PM, Eyal Lotem <span dir="ltr"><<a href="mailto:eyal.lotem@gmail.com" target="_blank">eyal.lotem@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'd like to propose a change in the behavior of Control.Exception to help guarantee cleanups are not accidentally lost.<div>
<br></div><div>For example, bracket is defined as:</div><div><pre style="color:rgb(0,0,0)"><span>bracket</span> <span>before</span> <span>after</span> <span>thing</span> <span style="color:red">=</span>
<a name="14840f85e66d9604_1483fa15ff2dddc6_1483f915830d8b99_1483df782fc59d78_1483d4e6f061948f_line-260"></a> <span>mask</span> <span>$</span> <span style="color:red">\</span><span>restore</span> <span style="color:red">-></span> <span style="color:blue">do</span>
<a name="14840f85e66d9604_1483fa15ff2dddc6_1483f915830d8b99_1483df782fc59d78_1483d4e6f061948f_line-261"></a> <span>a</span> <span style="color:red"><-</span> <span>before</span>
<a name="14840f85e66d9604_1483fa15ff2dddc6_1483f915830d8b99_1483df782fc59d78_1483d4e6f061948f_line-262"></a> <span>r</span> <span style="color:red"><-</span> <span>restore</span> <span style="color:red">(</span><span>thing</span> <span>a</span><span style="color:red">)</span> <span>`onException`</span> <span>after</span> <span>a</span>
<a name="14840f85e66d9604_1483fa15ff2dddc6_1483f915830d8b99_1483df782fc59d78_1483d4e6f061948f_line-263"></a> <span style="color:blue">_</span> <span style="color:red"><-</span> <span>after</span> <span>a</span>
<a name="14840f85e66d9604_1483fa15ff2dddc6_1483f915830d8b99_1483df782fc59d78_1483d4e6f061948f_line-264"></a> <span>return</span> <span>r</span></pre></div><div><div>This definition has a serious problem: "after a" (in either the exception handling case, or the ordinary case) can include interruptible actions which abort the cleanup.</div>
<div><br></div><div>This means bracket does not in fact guarantee the cleanup occurs.</div><div><br></div><div>For example:</div><div><br></div><div>readMVar = bracket takeMVar putMVar -- If async exception occurs during putMVar, MVar is broken!</div>
<div><br></div><div>withFile .. = bracket (openFile ..) hClose -- Async exception during hClose leaks the file handle!</div><div><br></div><div>Interruptible actions during "before" are fine (as long as "before" handles them properly). Interruptible actions during "after" are virtually always a bug -- at best leaking a resource, and at worst breaking the program's invariants.</div>
<div><br></div><div>I propose changing all the cleanup handlers to run under uninterruptibleMask, specifically:</div><div><br></div><div><b>bracket, bracketOnError, bracket_, catch, catchJust, finally, handle, handleJust, onException</b></div>
<div><b><br></b></div><div>should all simply wrap their exception/cancellation handler with uninterruptibleMask.</div><div><br></div><div>The danger of a deadlock is minimal when compared with the virtually guaranteed buggy incorrect handling of async exceptions during cleanup.</div>
<span><font color="#888888">
<div><br></div>-- <br><div dir="ltr">Eyal</div>
</font></span></div></div>
<br>_______________________________________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org" target="_blank">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>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Gregory Collins <<a href="mailto:greg@gregorycollins.net" target="_blank">greg@gregorycollins.net</a>>
</div>
</blockquote></div>
</div></div><br>_______________________________________________<br>
Libraries mailing list<br>
<a href="mailto:Libraries@haskell.org" target="_blank">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>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span class="HOEnZb"><font color="#888888">-- <br><div dir="ltr">Eyal</div>
</font></span></div>
</blockquote></div><br></div>