Final bikeshedding call: Fixing Control.Exception.bracket

[John Lato]

 On 01:46, Thu, Nov 13, 2014 Yuras Shumovich <shumovichy at gmail.com> wrote:

On Thu, 2014-11-13 at 02:28 +0000, John Lato wrote:
> On Thu Nov 13 2014 at 8:58:12 AM Yuras Shumovich <shumovichy at gmail.com>
> wrote:
> > You are wrong, hClose closes the handle in case of any exception, so
> > there is no leak here. I already described that and pointed to source
> > code. Probably my arguments are weak, but nobody even tried to argue the
> > opposite. The relevant part:
> >
>
> People have been arguing the opposite.  hClose is not guaranteed to close
> the handle in case an exception arises.  Here's a demonstration program.

Sorry, I mean that nobody argued that my analysis of hClose behavior is
wrong. I noted that hClose can leak in case of concurrent access to the
handle, but found that not an issue in practice. Until now nobody argued
the opposite.

> You might argue that hClose should use uninterruptibleMask internally
> (which is the only way to fix the issue).  Possibly so.  However, this is
a
> really pervasive problem, which is why it makes some sense to place the
> mask in bracket and fix every handler properly.

Yes, I argue that hClose should be fixed, not bracket. I don't have
strong opinion whether it should use uninterruptibleMask or handle it in
any other way.

>
> At some point in this thread a person (you?) has argued that this isn't a
> problem in practice.  I disagree.  It actually seems to be fairly common
in
> certain types of network programming.

(You are correct, it was me)

Ok, your example convinced me that it may be an issue in practice. But
I'm still not sure it *is* an issue in practice because nobody created
an issue about that.
Note: there were an issue (see here:
https://ghc.haskell.org/trac/ghc/ticket/3128 ) about resource leak
caused by sync exception inside hClose, but it was fixed.

Anyway, lets fix hClose, not bracket.

 hClose is a good example because it's pervasive and currently incorrect.
But it's not the only one, and furthermore a lot of broken code is likely
in libraries.

For the most part, Haskellers don't need to worry about async exceptions.
using them properly is a rather rare skill currently, and I suspect people
are basically happy with that.  But it's dangerous because people actually
do need to write async-safe code in cleanup handlers to get the behavior
that usually mean.   One reason I think changing bracket etc is a better
solution is that it helps minimize the average programmer's exposure to
async exceptions.  It becomes easier to write correct cleanup handlers, for
everyone. And if someone really needs an interruptible handler that can be
available by a specialized function. But bracket and catches aren't the
place for that.

> > > So sync exceptions in hClose mean the program is incorrect, and the
only
> > > recourse is to prevent the sync exceptions in the first place.
> > Fortunately,
> > > these FDs are likely guaranteed to be valid so sync exceptions are
> > > virtually ruled out.
> > >
> > > This is a general pattern with cleanups: a cleanup already has the
> > > allocated resource at hand, which almost always rules out sync
> > exceptions.
> > > Also, exceptions during an error-induced cleanup cause dangerous
> > > error-silencing anyway since we cannot handle an exception within an
> > > exception.
> >
> > So you have to inspect all the code, directly or indirectly used by
> > cleanup action, to ensure it doesn't throw sync exception (just to find
> > that it is not the case -- a lot of cleanup actions can throw sync
> > exceptions in some, probably rare, cases.) Someone argued, that was
> > exactly the issue the proposal was trying to solve.
> >
>
> Sync exceptions have nothing to do with the proposal.  The proposal itself
> certainly doesn't argue this.

Sorry, I was not clear enough. I'm referring to the next:

On Tue, 2014-11-11 at 12:17 -0800, Merijn Verstraaten wrote:
> Both Eyal and me have had trouble with this where we had to entire
> half of base and part of the runtime, to figure out whether our code
> was async exception safe. Auditing half the ecosystem to be able to
> write a safe cleanup handler is *NOT* a viable option.

By banning sync exceptions in cleanup action you require Merijn to audit
half the ecosystem to figure out whether his code is *sync* exception
safe. Which probably requires the same amount of work as inspecting code
for *async* exception safety.

 What? Nobody wants to ban sync exceptions in cleanups.  They should
continue to work the way they do now.  Why do you bring this up?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/libraries/attachments/20141113/0839e421/attachment.html>