Opened 9 years ago

Closed 7 years ago

#2558 closed bug (wontfix)

re-throwing an asynchronous exception throws it synchronously

Reported by: simonmar Owned by: simonmar
Priority: normal Milestone: 7.0.1
Component: Compiler Version: 6.8.3
Keywords: Cc: sclv, id@…
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

If you catch and re-throw an asynchronous exception, the second time it will be thrown synchronously. This only makes a difference when using unsafePerformIO or unsafeInterleaveIO, because otherwise the catch will not be inside any thunks.

This could have the unfortunate effect of updating an unsafePerformIO thunk with Timeout, or KillThread, or some other asychronous exception that should not be recorded as the value of the thunk.

We believe that exceptions thrown by an exception handler should be thrown in the same mode (synchronous or asynchronous) as the original exception. This means that we need to

  • keep track of the current exception-throwing mode
  • push a frame when in an exception handler (we already do this, because the exception handler is implicitly wrapped in block)
  • respect this mode when throwing exceptions.

This raises another issue: catching an exception and recovering by tail-calling out of the exception handler is not a good thing to do. Currently it has the undesirable effect that the thread remains in block mode, and an explicit unblock is required. With the changes above, the thread may also remain in "asynchronous exception" mode. The right way to solve this is just "don't do that". Here's how to use the exception API:

  • For cleanup, use finally, bracket, and onException.
  • To modify the exception, use mapException.
  • For recovery, use try.

Perhaps we should be deprecating catch? Or perhaps catch should be implemented in terms of try, so it doesn't have the implicit block / re-throw async exceptions behaviour?

Change History (15)

comment:1 Changed 9 years ago by simonmar

Owner: set to simonmar

comment:2 Changed 9 years ago by simonmar

Architecture: UnknownUnknown/Multiple

comment:3 Changed 9 years ago by simonmar

Operating System: UnknownUnknown/Multiple

comment:4 Changed 8 years ago by igloo

So if an exception handler (for an asynchronous exception) divides by zero, that would be an asynchronous exception, right? Is that what we want?

As an alternative, should we actually be throwing

data SomeExceptionSync = SomeExceptionSync Bool -- synchronous?
                                           SomeException

with catch etc ignoring the Bool, but bracket etc handling it appropriately? This would also mean that tail-calling out of an exception handler would work.

comment:5 Changed 8 years ago by int-e

My first instinct would be to pass a flag to exception handlers (changing the type of the catch# primitive, and the stack frame creation in RaiseAsync.c and Exception.cmm accordingly) to sort out at their leisure. Igloo's idea is similar in that would also pass a flag along with the exception, although the flag is set at a different point. I'm not sure which is easier to accomplish.

It's already possible to use such a flag:

throwAsync :: Exception e => e -> IO ()
throwAsync e = do
    target <- myThreadId
    throwTo target e

throwSyncOrAsync :: Exception e => Bool -> e -> IO ()
throwSyncOrAsync async = if async then throwAsync else throwIO

If such a flag is added, I'd also like to have a variant of catch that gets to see it in Control.Exception, say,

catchSyncOrAsync :: Exception e => IO a -> (Bool -> e -> IO a) -> IO a

and possibly catchSync and catchAsync.

comment:6 Changed 8 years ago by simonpj

See also this email thread which, I believe, boiled down to the same issue.

Simon

comment:7 in reply to:  4 Changed 8 years ago by simonmar

Replying to igloo:

So if an exception handler (for an asynchronous exception) divides by zero, that would be an asynchronous exception, right? Is that what we want?

Yes, I believe that's what we want. If the exception handler throws a divide-by-zero, and it did so as a result of an asynchronous exception (say ThreadKilled), then we don't want to update any thunks with the divide-by-zero exception, we want to revert them as for an asynchronous exception.

As an alternative, should we actually be throwing

data SomeExceptionSync = SomeExceptionSync Bool -- synchronous?
                                           SomeException

with catch etc ignoring the Bool, but bracket etc handling it appropriately? This would also mean that tail-calling out of an exception handler would work.

Tail-calling out of an exception handler also has the problem that it leaves the thread in the blocked (i.e. Control.Exception.blocked) state. IMO we should be using and recommending try rather than catch for handling exceptions as per the description in this bug.

I haven't put any more thought into the suggestions in this thread yet... will do later.

comment:8 in reply to:  5 Changed 8 years ago by simonmar

Replying to int-e:

My first instinct would be to pass a flag to exception handlers (changing the type of the catch# primitive, and the stack frame creation in RaiseAsync.c and Exception.cmm accordingly) to sort out at their leisure. Igloo's idea is similar in that would also pass a flag along with the exception, although the flag is set at a different point. I'm not sure which is easier to accomplish.

It's already possible to use such a flag:

throwAsync :: Exception e => e -> IO ()
throwAsync e = do
    target <- myThreadId
    throwTo target e

throwSyncOrAsync :: Exception e => Bool -> e -> IO ()
throwSyncOrAsync async = if async then throwAsync else throwIO

Good point - and so a partial workaround for this bug is to always throw asynchronous exceptions like ThreadKilled using throwAsync.

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

Cc: id@… added

Replying to int-e:

throwAsync :: Exception e => e -> IO () throwAsync e = do

target <- myThreadId throwTo target e

nitpick: what if throwAsync is called when in blocked mode? does it need to do unblock (throwTo target e) in order for the self-exception to arrive? (or maybe the RTS already side-steps this deadlock?)

-Isaac Dupree

comment:1 in reply to:  9 Changed 8 years ago by int-e

Replying to Isaac:

nitpick: what if throwAsync is called when in blocked mode? does it need to do unblock (throwTo target e) in order for the self-exception to arrive? (or maybe the RTS already side-steps this deadlock?)

Currently, the exception gets delivered, even in blocked mode.

There's actually a todo in Exception.cmm for that case:

    if (target == CurrentTSO) {
        SAVE_THREAD_STATE();
        /* ToDo: what if the current thread is blocking exceptions? */
        foreign "C" throwToSingleThreaded(MyCapability() "ptr", 
                                          target "ptr", exception "ptr")[R1,R2];

I'd say that the current behaviour is sensible: throwTo is a blocking operation. As soon as the thread blocks on itself, it can deliver its own exception. We may just as well skip the step of getting blocked.

comment:11 Changed 8 years ago by igloo

Milestone: 6.12 branch6.12.1

comment:12 Changed 8 years ago by igloo

Milestone: 6.12.16.14.1
Type of failure: None/Unknown

comment:13 Changed 7 years ago by sclv

Cc: sclv added

comment:14 Changed 7 years ago by simonmar

I realised while thinking about this recently that the proposal in the ticket doesn't make sense. We can't make throw :: Exception -> a throw an asynchronous exception, because its value is bottom; there's nothing sensible to do on resumption other than throw the same exception. Similarly, it doesn't make sense for something like finally or onException to re-throw an exception asynchronously, because we have no way to decide what to do when the computation is resumed (e.g. inside unsafePerformIO).

You can code up re-throwing of asynchronous exceptions manually using throwTo to throw to the current thread, which works fine because on resumption the behaviour is just as if throwTo had returned, but the caller has to have a plan for what to do in this event - which might be redoing the original IO operation, or it might be something else.

See #3997 for an instance of this issue that affects lazy I/O.

comment:15 Changed 7 years ago by simonmar

Resolution: wontfix
Status: newclosed

Closing this ticket (see previous comment); I don't think there's anything to be done here.

Note: See TracTickets for help on using tickets.