Re: Final bikeshedding call: Fixing Control.Exception.bracket

On Thu, 2014-11-13 at 02:28 +0000, John Lato wrote:

On Thu Nov 13 2014 at 8:58:12 AM Yuras Shumovich 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.

...

...

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. Note: users already relies on hClose doesn't leak in case of exception (see Trac #3128 I pointed to above) (hClose does try to guaranty that). So we can't just ban sync exceptions in cleanup without deeper investigation. Thanks, Yuras