getChanContents exception behavior seems a bit odd

I've been playing around with Control.Concurrent.Chan today. Something seems a bit off:

-- Bug.hs
import Control.Concurrent (forkIO, yield)
import Control.Concurrent.Chan
import Data.List (elem)
import Control.Exception
import Control.Concurrent.MVar

data Ex = Ex deriving Show
instance Exception Ex

main = do
  ch <- newChan
  sync1 <- newEmptyMVar
  sync2 <- newEmptyMVar
  forkIO $ do {writeList2Chan ch [1..3*10^6 :: Int]; putMVar sync1 ()}
  yield
  writeChan ch (-12)
  cont <- getChanContents ch

  tid <- forkIO $ do
           evaluate (last cont)
           putMVar sync2 ()

  yield
  throwTo tid Ex


  print (elem (3*10^6) cont)
  takeMVar sync1
  tryTakeMVar sync2

When I run this single-threaded (+RTS -N1), it prints

Bug: Ex
Bug: Ex

One of the thunks in the lazy list gets overwritten by the (asynchronous) exception. This seems a bit surprising; is it the way it should be? Does hGetContents do this too?

Trac metadata

Trac field	Value
Version	8.4.3
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Core Libraries
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	simonmar
Operating system
Architecture

changed milestone to %8.6.1

changed weight to 5

added Tbug Trac import core libraries labels

The basic problem seems to be that readChan uses modifyMVar, which uses onException, which catches arbitrary exceptions and rethrows them as synchronous exceptions:

onException :: IO a -> IO b -> IO a
onException io what = io `catch` \e -> do _ <- what
                                          throwIO (e :: SomeException)

I don't think readChan can throw any synchronous exceptions, so perhaps it should rethrow any exceptions it receives asynchronously and retry.

Yeah, this is tricky stuff. We have special handling for async exceptions in the IO library, see Note [async] in GHC.IO.Handle.Internals.

Note that since you threw Ex as an async exception, it should really be a child of AsyncException in the exception hierarchy, because we use that assumption when handling async exceptions differently. (it wouldn't have made a difference with your code though).

changed milestone to %8.8.1

These won't be fixed for in GHC 8.6.

changed milestone to %8.10.1

Bumping milestones of low-priority tickets.

added incorrect runtime result label

added Pnormal label

removed milestone