Ticket #2363 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

getChar cannot be interrupted with -threaded

Reported by: simonmar Owned by:
Priority: high Milestone: 6.10 branch
Component: libraries/base Version: 6.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

Now that stdin, stdout and stderr are left in blocking mode, when using the threaded RTS, I/O operations on these handles are no longer interruptible. We ought to do something about this.

module Main where

import Control.Monad
import System.Posix.Signals
import Control.Concurrent
import Control.Concurrent.MVar

import System.IO

main = do
    hSetBuffering stdin NoBuffering
    hSetEcho stdin False
    mv <- newEmptyMVar
    let handler = putMVar mv Nothing
    installHandler sigINT (CatchOnce handler) Nothing
    tid <- forkIO (myGetChar mv)
    c <- takeMVar mv
    when (c==Nothing) $ do
        killThread tid
    putStrLn ("Result: " ++ show c)

myGetChar mv = do
    c <- getChar
    putMVar mv (Just c)

Change History

follow-up: ↓ 2   Changed 5 years ago by judah

I was the original reporter of this problem. I have worked around it by rewriting myGetChar:

myGetChar mv = do
    Control.Concurrent.threadWaitRead System.Posix.IO.stdInput
    c <- getChar
    putMVar mv (Just c)

The problem with the original code was that getChar (and hWaitForInput) wrap foreign calls, which block the subthread they're run in and thus cannot be halted by killThread. In contrast, threadWaitRead wraps a primop, so it does not have that problem.

This workaround is OK for now, but in my opinion all of the IO functions should be interruptible by default. (And threadWaitRead uses Fd instead of Handle which is more convenient/portable.)

in reply to: ↑ 1 ; follow-up: ↓ 3   Changed 5 years ago by simonmar

Replying to judah:

I was the original reporter of this problem. I have worked around it by rewriting myGetChar: {{{ myGetChar mv = do Control.Concurrent.threadWaitRead System.Posix.IO.stdInput c <- getChar putMVar mv (Just c) }}}

That's ok, as long as another thread doesn't call getChar right after your threadWaitRead, then your getChar will hang again (not likely, I imagine, but I've trained myself to spot race conditions :-).

Another workaround is to put stdin into non-blocking mode,

import System.Posix
... setFdOption stdInput NonBlockingRead True

but then you risk problems that caused us to stop doing this in the first place (#724).

The problem with the original code was that getChar (and hWaitForInput) wrap foreign calls, which block the subthread they're run in and thus cannot be halted by killThread. In contrast, threadWaitRead wraps a primop, so it does not have that problem.

Actually in the threaded RTS threadWaitRead is not a primop, it's a communication with the IO manager thread.

This workaround is OK for now, but in my opinion all of the IO functions should be interruptible by default. (And threadWaitRead uses Fd instead of Handle which is more convenient/portable.)

I'm surprised you think that FD is more portable. Windows doesn't have FDs except in a compatibility layer.

in reply to: ↑ 2 ; follow-up: ↓ 4   Changed 5 years ago by judah

Replying to simonmar:

That's ok, as long as another thread doesn't call getChar right after your threadWaitRead, then your getChar will hang again (not likely, I imagine, but I've trained myself to spot race conditions :-).

I think you can prevent that race by calling hGetBufNonBlocking, and retrying if it doesn't read a character:

import qualified Data.ByteString.Char8 as B
myGetChar mv = do
    threadWaitRead stdInput
    bs <- B.hGetNonBlocking stdin 1
    if B.null bs
        then myGetChar mv
        else putMVar mv (Just (B.head bs))

There's a potential (again, unlikely) starvation problem in that this thread could keep getting preempted by another thread reading stdin; but for my particular application, I'm much more concerned with interruptibility than fairness.

The problem with the original code was that getChar (and hWaitForInput) wrap foreign calls, which block the subthread they're run in and thus cannot be halted by killThread. In contrast, threadWaitRead wraps a primop, so it does not have that problem.

Actually in the threaded RTS threadWaitRead is not a primop, it's a communication with the IO manager thread.

My mistake; I didn't read the sources carefully enough.

This workaround is OK for now, but in my opinion all of the IO functions should be interruptible by default. (And threadWaitRead uses Fd instead of Handle which is more convenient/portable.)

I'm surprised you think that FD is more portable. Windows doesn't have FDs except in a compatibility layer.

Sorry, I worded that poorly. I meant of course that Handle is more convenient/portable than Fd.

in reply to: ↑ 3   Changed 5 years ago by simonmar

Replying to judah:

I think you can prevent that race by calling hGetBufNonBlocking, and retrying if it doesn't read a character

Ironically enough, hGetBufNonBlocking suffers from similar problems. As far as I know it's impossible to implement in a completely race-free way without using O_NONBLOCK, because you have to make two system calls: first select() then read(), and someone else could read() between those two. Fortunately GHC almost avoids the race by holding the Handle lock, which is ok as long as all clients are using Handles instead of raw FDs.

Anyway, I have a fix that I'm testing now; it's along the same lines as your suggestion (call select() first, then read()).

  Changed 5 years ago by simonmar

  • status changed from new to closed
  • resolution set to fixed

Fixed:

Thu Jun 19 07:19:11 PDT 2008  Simon Marlow <marlowsd@gmail.com>
  * Fix #2363: getChar cannot be interrupted with -threaded

  Changed 5 years ago by simonmar

  • architecture changed from Unknown to Unknown/Multiple

  Changed 5 years ago by simonmar

  • os changed from Unknown to Unknown/Multiple
Note: See TracTickets for help on using tickets.