Opened 9 years ago

Closed 9 years ago

#4810 closed proposal (wontfix)

Versions of bracket without masking acquire action

Reported by: mitar Owned by:
Priority: normal Milestone: 7.4.1
Component: libraries/base Version: 7.0.1
Keywords: Cc: mmitar@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

With new mask/restore concept it is impossible to unblock exceptions in acquire action which is quite often useful (acquire can be a lengthily action). So it could be a good idea to introduce a variant which has acquire action unmasked.

Of course I could write:

mask $ \restore -> bracket (restore . open) (close) (restore . work)

but this is just ugly.

So I am proposing bracket' or bracketUnmasked or something.

Change History (12)

comment:1 Changed 9 years ago by simonmar

I don't see how you could use that safely. bracket only works when the acquire action is atomic - either it acquires the resource and returns, or it does not acquire the resource and raises an exception. If the acquire action is unmasked, then it could both acquire the resource and raise an exception, and the resource would not be released.

If you have an example of a safe use, could you show me the code?

comment:2 Changed 9 years ago by mitar

I agree with you.

As I see there are two possibilities. We can add a version of bracket where actions gets as an argument restore function. So that they can restore it inside, for example they can have internally more brackets, which make sure things are not acquired if exception is raised, but that exception can still be raised in a lengthy acquire.

The other, which I opted for above, is that we unmask acquire and then user can call mask immediately at the beginning of the acquire action, thus having a restore for use somewhere later. This short period of non-masked state is not a problem as nothing has yet been really acquired at the beginning of the acquire action.

My code where I require something like this is (simplified):

bracketOnErrorUnmasked createThread killThread doSomethingWithThread

createThread = mask $ \restore -> do
  -- do some preparations
  forkIO $ bracket prepareThread cleanupThreadAndFillMVar (restore doThreadAction)

In original code there is then also a Chan for interaction with this created thread in doSomethingWithThread, but this is not important. bracketOnErrorUnmasked is a version of bracketOnError without masked acquire action.

forkIOUnmasked does not help here because then there is a short period of time when cleanupThreadAndFillMVar will not be called (before internal bracket is initialized) so my program could hang waiting for MVar from a thread. And I want doThreadAction to be unmasked as otherwise thread is killable only in interruptible actions.

comment:3 in reply to:  2 ; Changed 9 years ago by simonmar

Replying to mitar:

As I see there are two possibilities. We can add a version of bracket where actions gets as an argument restore function. So that they can restore it inside, for example they can have internally more brackets, which make sure things are not acquired if exception is raised, but that exception can still be raised in a lengthy acquire.

So you're saying you want something like:

  bracket (do bracket a b c; d) e f

but can't you just transform that to

  do bracket a b c
     bracket d e f

?

The other, which I opted for above, is that we unmask acquire and then user can call mask immediately at the beginning of the acquire action, thus having a restore for use somewhere later. This short period of non-masked state is not a problem as nothing has yet been really acquired at the beginning of the acquire action.

That wouldn't be safe either, because there would be a window at the end of the acquire action with exceptions unblocked.

My code where I require something like this is (simplified):

bracketOnErrorUnmasked createThread killThread doSomethingWithThread

createThread = mask $ \restore -> do
  -- do some preparations
  forkIO $ bracket prepareThread cleanupThreadAndFillMVar (restore doThreadAction)

Right, so here you can receive an exception after the forkIO, and doSomethingWithThread will not be executed.

forkIOUnmasked does not help here because then there is a short period of time when cleanupThreadAndFillMVar will not be called (before internal bracket is initialized) so my program could hang waiting for MVar from a thread. And I want doThreadAction to be unmasked as otherwise thread is killable only in interruptible actions.

Correct. I now realise that forkIOUnmasked is not really what we wanted; what we really should have added was

forkIOWithUnmask :: ((forall a. IO a -> IO a) -> IO a) -> IO ThreadId

Wouldn't that solve your problem?

comment:4 in reply to:  3 ; Changed 9 years ago by mitar

Replying to simonmar:

but can't you just transform that to

I do not see a way. This is an example from a library where user can use bracket to wrap my library calls but then I am "unable" to use bracket inside my library (and I want to use bracket because I want that my function to be interruptible). What I want to do is to tell the user that best practice is to use unmasked version of a bracket when using my library because I do take care of cleaning myself. But of course user should use bracket because it should cleanup things. This is somewhat a motivation behind all this. How to give an user a simple suggestion what to use when calling my library. It should be an acquire-use-release approach, but I would like to have acquire interruptible for the user (at least in my best practice suggestion, user could still of course mask everything if this is the best for him/her, but currently user does not have this option to choose, it is always masked with bracket). And I also want this acquire and release parts to be available separately to the user.

Maybe I have to do one step back and rethink everything. I started from bracket and now I am trying to modify a bracket to suit my needs by maybe it should be simply something else to begin with.

That wouldn't be safe either, because there would be a window at the end of the acquire action with exceptions unblocked.

True. It seems the correct approach is not that whole acquire action is unmasked, but that somewhere internally it can be unmasked (where acquire action can make sure everything is correctly handled).

Right, so here you can receive an exception after the forkIO, and doSomethingWithThread will not be executed.

This is not the problem. The problem is that killThread is not executed.

Wouldn't that solve your problem?

Yes. In my particular instance it would. Please add also a version for forkOnIO.

But I still argue that we need also:

bracketWithUnmask :: ((forall a. IO a -> IO a) -> IO a) -> (a -> IO b) -> (a -> IO c) -> IO c

Although it seems it will not be useful in my case as I would have to change type signature for acquire function.

comment:5 in reply to:  4 ; Changed 9 years ago by simonmar

Replying to mitar:

Replying to simonmar:

but can't you just transform that to

I do not see a way. This is an example from a library where user can use bracket to wrap my library calls but then I am "unable" to use bracket inside my library (and I want to use bracket because I want that my function to be interruptible).

Ok, so to be clear you want to provide a library that exposes acquire and release operations, which you expect the user to be able to use in bracket. Furthermore, within acquire, you want to use bracket yourself to acquire and release some other resource.

Remember that acquire will still be interruptible, since it is within mask, not uninterruptibleMask. Is there really a problem here?

But I still argue that we need also:

bracketWithUnmask :: ((forall a. IO a -> IO a) -> IO a) -> (a -> IO b) -> (a -> IO c) -> IO c

But that breaks the abstraction - you shouldn't be able to unmask unconditionally. That's why we moved to mask instead of block/unblock. The exception to the rule is for forkIO, where it is harmless to unmask (and necessary, in some cases).

comment:6 in reply to:  5 ; Changed 9 years ago by mitar

Replying to simonmar:

Remember that acquire will still be interruptible, since it is within mask, not uninterruptibleMask. Is there really a problem here?

Yes, for example I have a lengthy acquire which calls FFI. Code in FFI is not interruptible. And code calling it and preparing does not use any IO actions, MVars...

Maybe we could introduce an interruptible no-op IO operation which would just allow that at that moment exception is thrown even if the section is masked. Maybe yield is already this? Or no-op with cleanup argument to call in a case of exception.

But that breaks the abstraction - you shouldn't be able to unmask unconditionally. That's why we moved to mask instead of block/unblock.

Buy you can unmask only if you are allowed to. So if I am developer of the code which is called with such bracketWithUnmask then I know that caller allows me to unmask as necessary. And vice versa, if I am calling with bracketWithUnmask I know that I am allowing the code I am calling to unmask. This is much much different to the situation before, where you could not make any assumption about this and you could not make a contract/API which would define this.

So yes, we introduced mask because you should not be able to unmask if not allowed to. And before you could. There was no other mechanism in place. Currently, there is: do you have a restore function (a token, a capability) or not.

Look what we do internally in our code: we mask and then pass restore to those parts where we want to allow them to unmask. But why would we limit this only to the code somebody writes him/herself. Why shouldn't this be modular in a sense that masking is done somewhere and unmasking is done somewhere else, in somebody else's code/module. It is just about modularity. If I would wrote all code by myself I would simply pass it around as necessary. But now I want to make a contract/API to my module so that things can be modular.

And if somebody does not want to give my module power to restore he/she can simply give me id function.

comment:7 in reply to:  6 ; Changed 9 years ago by simonmar

Replying to mitar:

Replying to simonmar:

Remember that acquire will still be interruptible, since it is within mask, not uninterruptibleMask. Is there really a problem here?

Yes, for example I have a lengthy acquire which calls FFI. Code in FFI is not interruptible. And code calling it and preparing does not use any IO actions, MVars...

FFI isn't interruptible whether you're inside mask or not.

Maybe we could introduce an interruptible no-op IO operation which would just allow that at that moment exception is thrown even if the section is masked.

Now that's a very good idea. How about

allowInterrupt :: IO ()

But that breaks the abstraction - you shouldn't be able to unmask unconditionally. That's why we moved to mask instead of block/unblock.

Buy you can unmask only if you are allowed to. So if I am developer of the code which is called with such bracketWithUnmask then I know that caller allows me to unmask as necessary. And vice versa, if I am calling with bracketWithUnmask I know that I am allowing the code I am calling to unmask. This is much much different to the situation before, where you could not make any assumption about this and you could not make a contract/API which would define this.

But consider a library function that uses bracketWithUnmask. If it is called inside a mask, then it should not be able to unmask. This was the problem that we fixed with mask: that's why there is no unmask operation any more.

comment:8 in reply to:  7 ; Changed 9 years ago by mitar

Replying to simonmar:

FFI isn't interruptible whether you're inside mask or not.

I was not saying otherwise. ;-) Just stating how you can get a lengthy acquire.

Now that's a very good idea.

In fact I do not really like it. ;-)

And I do not really see much difference between allowInterrupt and unblock yield. You still unmask where you maybe should not (from a caller point of view). But yes, you couldn't really call something else in a unblocked context. That is different.

But consider a library function that uses bracketWithUnmask. If it is called inside a mask, then it should not be able to unmask. This was the problem that we fixed with mask: that's why there is no unmask operation any more.

Yes, it cannot unmask. The same as normal bracket cannot unmask in-between action if it is called in masked context. There is nothing wrong with that. What I am arguing for is that if the caller wants to allow acquire not to be masked, he/she can:

  1. call it in an unmasked context
  2. call it with bracketWithUnmask (sadly, this requires that calling function have a corresponding type signature, but this is probably also a good thing as it says that calling function can take advantage of being able to unmask)

I caller does not want to allow unmasking, he/she can:

  1. mask
  2. pass id to bracketWithUnmask (or use a variant of calling function without restore argument)

And this is the same for library function or for user function. restore function might not do anything as mask was called already in a masked context. But this is OK.

I am only saying there should be a function in a standard library so that you do not need to write your own. I think it is completely safe (it does not introduce unmasking where it should/could not). And I argue that it is useful for situations where you have lengthy acquire action where you can handle interrupts (like you should handle them because maybe you are also using interruptible actions).

comment:9 in reply to:  8 ; Changed 9 years ago by simonmar

Replying to mitar:

Replying to simonmar:

FFI isn't interruptible whether you're inside mask or not.

I was not saying otherwise. ;-) Just stating how you can get a lengthy acquire.

But it wouldn't make a difference if it was unmasked, it would still not be interruptible.

Now that's a very good idea.

In fact I do not really like it. ;-)

And I do not really see much difference between allowInterrupt and unblock yield.

There's no difference semantically. unblock yield is an implementation of allowInterrupt, but a better implementation is just unblock (return ()).

You still unmask where you maybe should not (from a caller point of view).

Not true - any operation can potentially raise asynchronous exceptions, by being interruptible. This is just a way to be interruptible without actually invoking an interruptible operation, which all involve blocking. I like it!

I am only saying there should be a function in a standard library so that you do not need to write your own.

Then I misunderstood what you wanted blockWithUnmask to do. Could you show me the code?

comment:10 in reply to:  9 Changed 9 years ago by mitar

Replying to simonmar:

But it wouldn't make a difference if it was unmasked, it would still not be interruptible.

Yes.

I like it!

;-)

Then I misunderstood what you wanted blockWithUnmask to do. Could you show me the code?

bracketWithUnmask, not blockWithUnmask (just to be sure).

bracketWithUnmask before after thing =
  mask $ \restore -> do
    a <- before restore
    r <- restore (thing a) `onException` after a
    _ <- after a
    return r

This. ;-) Before just gets additional argument: restore.

To be clear, this was not my initial proposal, for that you showed it would not work correctly. My initial proposal would have restore before there.

Of course the same argument could be applied also that after should also have restore as an argument, but I cannot really imagine a real-world use for this. And it is probably then easier to write everything by hand with mask than using some variant of bracket.

And if user does not want to allow unmasking, then it is possible to just do:

bracket (before id) after ...

Of course this is still doable very similar to my initial approach:

mask $ \restore -> bracket (open restore) (close) (restore . work)

comment:11 Changed 9 years ago by igloo

Milestone: 7.2.1

comment:12 Changed 9 years ago by simonmar

Resolution: wontfix
Status: newclosed

See #4857 for allowInterrupt, and #4858 for forkIOWithUnmask. I think we can close this ticket now, please reopen if you disagree.

Note: See TracTickets for help on using tickets.