Opened 3 years ago

Closed 20 months ago

Last modified 19 months ago

#9516 closed bug (fixed)

unsafeUnmask unmasks even inside uninterruptibleMask

Reported by: edsko Owned by: simonmar
Priority: normal Milestone: 8.0.1
Component: Runtime System Version: 7.8.2
Keywords: Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D181
Wiki Page:

Description

Control.Exception exports

allowInterrupt :: IO ()
allowInterrupt = unsafeUnmask $ return ()

with documentation:

When invoked inside mask, this function allows a blocked asynchronous exception to be raised, if one exists. It is equivalent to performing an interruptible operation, but does not involve any actual blocking. When called outside mask, or inside uninterruptibleMask, this function has no effect.

However, this is not actually true: unsafeUnmask unmasks exceptions even inside uninterruptibleUnmask, as the attached test demonstrates (the test uses a foreign call just to have something non-interruptible but still observable; in particular, doing a print is interruptible because it uses an MVar under the hood).

I think it is possible to define a better unsafeUnmask in user-land:

interruptible :: IO a -> IO a
interruptible act = do
  st <- getMaskingState
  case st of
    Unmasked              -> act
    MaskedInterruptible   -> unsafeUnmask act
    MaskedUninterruptible -> act

but it still seems to be that we should either (i) change the behaviour of unsafeUnmask, or (ii) provide a version of unsafeUnmask with the behaviour as described and then change allowInterrupt to use that new version of unsafeUnmask, or at the very least (iii) change the documentation.

(One question with the above definition of interruptible is what happens when we nest mask and uninterruptibleMask?)

Attachments (2)

TestUnsafeUnmask.hs (902 bytes) - added by edsko 3 years ago.
printInt.c (66 bytes) - added by edsko 3 years ago.

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by edsko

Attachment: TestUnsafeUnmask.hs added

Changed 3 years ago by edsko

Attachment: printInt.c added

comment:1 Changed 3 years ago by simonmar

good catch, and a nice bug report. I think unsafeUnmask is fine, but the implementation of allowInterrupt is wrong, and we should replace it with your version. Would you like to submit a patch? (I'm on holiday right now, but no need to wait for me to approve it, Austin can approve and push.)

comment:2 Changed 3 years ago by thoughtpolice

Differential Rev(s): Phab:D181

comment:3 Changed 20 months ago by Ben Gamari <ben@…>

In 5a8a8a64/ghc:

Don't allowInterrupt inside uninterruptibleMask

This fixes #9516.

Differential Revision: https://phabricator.haskell.org/D181

Authored-by: Edsko de Vries <edsko@well-typed.com>

comment:4 Changed 20 months ago by bgamari

Resolution: fixed
Status: newclosed

comment:5 Changed 19 months ago by bgamari

Milestone: 8.0.1
Note: See TracTickets for help on using tickets.