Opened 7 years ago

Last modified 8 weeks ago

#5797 new bug

readRawBufferPtr cannot be interrupted by exception on Windows with -threaded

Reported by: joeyadams Owned by: Phyx-
Priority: normal Milestone: 8.10.1
Component: Core Libraries Version: 7.2.2
Keywords: Cc: ryan.gl.scott@…, hvr, ekmett
Operating System: Windows Architecture: x86
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: #11394 Differential Rev(s):
Wiki Page:

Description

On Windows, in a program compiled with -threaded, if a thread receives from a Handle created by the network package (e.g. with hGetLine), it cannot be interrupted by an asynchronous exception.

I've traced it down to readRawBufferPtr, which is used by Network.Socket.recv. Although I haven't tested readRawBufferPtr directly, my test case (attached) tests Network.Socket.recv.

For Windows, base 4.4.1.0 implements it as:

readRawBufferPtr :: String -> FD -> Ptr Word8 -> Int -> CSize -> IO CInt
readRawBufferPtr loc !fd buf off len
  | threaded  = blockingReadRawBufferPtr loc fd buf off len
  | otherwise = asyncReadRawBufferPtr    loc fd buf off len

...

blockingReadRawBufferPtr :: String -> FD -> Ptr Word8 -> Int -> CSize -> IO CInt
blockingReadRawBufferPtr loc fd buf off len
  = fmap fromIntegral $ throwErrnoIfMinus1Retry loc $
        if fdIsSocket fd
           then c_safe_recv (fdFD fd) (buf `plusPtr` off) len 0
           else c_safe_read (fdFD fd) (buf `plusPtr` off) len

...

-- NOTE: "safe" versions of the read/write calls for use by the threaded RTS.
-- These calls may block, but that's ok.

foreign import stdcall safe "recv"
   c_safe_recv :: CInt -> Ptr Word8 -> CSize -> CInt{-flags-} -> IO CSsize

foreign import stdcall safe "send"
   c_safe_send :: CInt -> Ptr Word8 -> CSize -> CInt{-flags-} -> IO CSsize

If I understand correctly, safe foreign calls cannot be interrupted by asynchronous exceptions.

Is this a bug in readRawBufferPtr, or a bug in the network package? Can a caller expect readRawBufferPtr to be interruptible by an exception?

Attachments (2)

kill-recv-raw.hs (1.3 KB) - added by joeyadams 7 years ago.
Works incorrectly when compiled with -threaded on Windows
threadWaitRead.hs (2.1 KB) - added by joeyadams 7 years ago.
threadWaitRead does something on Windows, but I don't know what

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by joeyadams

Attachment: kill-recv-raw.hs added

Works incorrectly when compiled with -threaded on Windows

comment:1 Changed 7 years ago by simonmar

difficulty: Unknown

Similar to #3937.

The real problem is that there is no IO manager on Windows. It is the communication between the client thread and the IO manager that allows IO operations to be interrupted on Unix systems, whereas on Windows with -threaded all IO is implemented with safe foreign calls.

comment:2 Changed 7 years ago by joeyadams

What if readRawBufferPtr simply called threadWaitRead before making the safe foreign call? I suppose there's a theoretical risk the foreign call may block after that, but it seems far less likely after a successful return from threadWaitRead.

Does threadWaitRead work on Windows with -threaded ?

comment:3 Changed 7 years ago by simonmar

No, threadWaitRead is not implemented on Windows with -threaded.

You can use a simple Async wrapper:

data Async a = Async (MVar a)

async :: IO a -> IO (Async a)
async action = do
   var <- newEmptyMVar
   forkIO (action >>= putMVar var)
   return (Async var)

wait :: Async a -> IO a
wait (Async var) = readMVar var

the problem here is that cancelling the wait call will not cancel the actual IO operation. That might not matter, depending on the details of your particular situation. FWIW, this is what happens in the non-threaded RTS, except that the behaviour is built into the RTS.

comment:4 Changed 7 years ago by joeyadams

No, threadWaitRead is not implemented on Windows with -threaded

What does threadWaitRead do on Windows (with -threaded) anyway? I wrote a test program that calls threadWaitRead on a socket without available data. It does not wake up when the server sends data, but it does accept an asynchronous exception.

When I use recv instead, the opposite happens. Namely, recv unblocks when data is sent from the server, but cannot be stopped by an asynchronous exception.

Changed 7 years ago by joeyadams

Attachment: threadWaitRead.hs added

threadWaitRead does something on Windows, but I don't know what

comment:5 Changed 7 years ago by simonmar

Ah, so I forgot that we did try to make threadWaitRead do something on Windows. Here's its implementation:

threadWaitRead :: Fd -> IO ()
threadWaitRead fd
#ifdef mingw32_HOST_OS
  -- we have no IO manager implementing threadWaitRead on Windows.
  -- fdReady does the right thing, but we have to call it in a
  -- separate thread, otherwise threadWaitRead won't be interruptible,
  -- and this only works with -threaded.
  | threaded  = withThread (waitFd fd 0)
  | otherwise = case fd of
                  0 -> do _ <- hWaitForInput stdin (-1)
                          return ()
                        -- hWaitForInput does work properly, but we can only
                        -- do this for stdin since we know its FD.
                  _ -> error "threadWaitRead requires -threaded on Windows, or use System.IO.hWaitForInput"
#else
  = GHC.Conc.threadWaitRead fd
#endif

withThread :: IO a -> IO a
withThread io = do
  m <- newEmptyMVar
  _ <- mask_ $ forkIO $ try io >>= putMVar m
  x <- takeMVar m
  case x of
    Right a -> return a
    Left e  -> throwIO (e :: IOException)

waitFd :: Fd -> CInt -> IO ()
waitFd fd write = do
   throwErrnoIfMinus1_ "fdReady" $
        fdReady (fromIntegral fd) write iNFINITE 0

iNFINITE :: CInt
iNFINITE = 0xFFFFFFFF -- urgh

foreign import ccall safe "fdReady"
  fdReady :: CInt -> CInt -> CInt -> CInt -> IO CInt

And we can see why it doesn't work with a socket: the 4th argument to fdReady should be non-zero for a socket, but we're always passing zero here, because we have no information about whether the Fd passed to threadWaitRead is a socket or not.

You could build your own version of threadWaitRead that works for sockets quite easy by modifying the above code.

comment:6 Changed 7 years ago by igloo

Milestone: _|_

comment:7 Changed 4 years ago by RyanGlScott

Cc: ryan.gl.scott@… hvr ekmett added

comment:8 Changed 4 years ago by thoughtpolice

Component: libraries/baseCore Libraries
Owner: set to ekmett

Moving over to new owning component 'Core Libraries'.

comment:9 Changed 3 years ago by thomie

Owner: ekmett deleted

comment:10 Changed 6 months ago by Phyx-

Milestone: 8.8.1
Owner: set to Phyx-

Will also require patches to network.

comment:11 Changed 8 weeks ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.