Final bikeshedding call: Fixing Control.Exception.bracket

Mon Nov 17 04:56:51 UTC 2014

+1 to the edward kmett clarified proposal.

On Fri, Nov 14, 2014 at 5:17 PM, Merijn Verstraaten <merijn at inconsistent.nl>
wrote:

>  Hi Edward,
>
> I think both me and Eyal where taking the addition of
> "interruptibleBracket" and variants for granted as an obvious part of this
> proposal. I fully agree that there *should* be an interruptibleBracket for
> people who want it. I just think it's the wrong default and that 'bracket'
> should become the obvious default.
>
> --
> Merijn
>
>
>
> On Fri, Nov 14, 2014, at 10:30, Edward Kmett wrote:
>
> It strikes me that this is winding up highly controversial.
>
> Assuming that, well, things might not go your way in terms of getting
> bracket changed, let's think a bit about what a more retrenched solution
> would look like.
>
> This way we have a continuum of fixes and can try to find the right point
> in the continuum.
>
> So let's put forth a couple more colors for the bikeshed:
>
> *Documentation Only*
>
> The issue is that users use bracket 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!
>
> We could document the pattern of using uninterruptibleMask_ yourself in
> handlers for safety and include examples of where it is required.
>
> *Combinators*
>
> If we want to go further, we could introduce:
>
> uninterruptibleBracket :: IO a -> (a -> IO b) -> (a -> IO c) -> IO c
> uninterruptibleBracket acquire release = bracket acquire
> (uninterruptibleMask_ . release)
>
> uninterruptibleBracket_ :: IO a -> IO b -> IO c -> IO c
> uninterruptibleBracket_ acquire release = bracket_ acquire
> (uninterruptibleMask_ release)
>
> etc.
>
> and then upgrade the documentation for `bracket` and the like with a big
> fat warning about how certain common sense examples using bracket should
> really be using uninterruptibleBracket.
>
> *Merijn's Proposal*
>
> We could go further and switch the default behavior of bracket to that of
> uninterruptibleBracket above. Then we get Merijn's proposal.
>
> *Going Further*
>
> 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`.
>
> The use cases that folks have that center around weird RPC handling
> scenarios in the release handler seem to fit into this niche.
>
> This redefines bracket, like Merijn would prefer, 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.
>
> It also has the benefit that the name interruptibleBracket is easier to
> explain than uninterruptibleBracket, which only made release
> uninterruptible.
>
> 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.
>
> *Personal Thoughts*
>
> I started writing this proposal with a continuum in mind, thinking I'd
> land somewhere in the middle.
>
> Normally, I'd be disinclined to change semantics on a function with such
> widespread use! I very strongly sympathize with Greg's position here.
>
> However, personally, I think the "Going Further" solution above is the
> right solution, which is effectively Merijn's proposal with the addition of
> interruptibleBracket and the like.
>
>  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 bracket to interruptibleBracket, 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.
>
> This effectively puts me at +1 with the caveat that I'd like to see
> interruptibleBracket added.
>
> -Edward
>
>
> On Tue, Nov 11, 2014 at 1:09 PM, Merijn Verstraaten <
> merijn at inconsistent.nl> wrote:
>
> Ola!
>
> 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.
>
> The problem, summarised:
> Blocking cleanup actions can be interrupted, causing cleanup not to happen
> and potentially leaking resources.
>
> Main objection to making the cleanup handler uninterruptible:
> Could cause deadlock if the code relies on async exceptions to interrupt a
> blocked thread.
>
> 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".
>
> The rest seems either +1, or at least agrees that the status quo is
> *worse* than the proposed solution.
>
> My counter to these objections is:
> 1) No one has yet shown me any code that relies on the cleanup handler
> being interruptible
>
> 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.
>
> 3) Even GHC-HQ can't use bracket correctly (see Simon's emails)
>
> Potential solution #1:
> Leave bracket as-is, add bracketUninterruptible with an uninterruptible
> cleanup handler.
>
> Potential solution #2:
> Change bracket to use uninterruptible cleanup handler, add
> bracketInterruptible for interruptible cleanups.
>
> Trade-offs:
> 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.
>
> 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.
>
> 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.
>
> 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.
>
> [1] -
> https://www.haskell.org/pipermail/libraries/2014-September/023675.html
>
> Cheers,
> Merijn
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries
>
>
>
>
>
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/libraries/attachments/20141116/1e676618/attachment-0001.html>