ghc-cabal deadlocks

[igloo]

Trying to build an OS X installer is currently deadlocking when

inplace/bin/ghc-cabal install /Users/ian/ghc/6.12-branch/val/dest/Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925/ghc-stage2 /Users/ian/ghc/6.12-branch/val/dest/Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925/ghc-pkg /Users/ian/ghc/6.12-branch/val/dest/Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925 compiler stage2 /Users/ian/ghc/6.12-branch/val/dest /Users/ian/ghc/6.12-branch/val/inst /Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925 /Users/ian/ghc/6.12-branch/val/inst/share/doc/ghc/html/libraries

runs:

/Users/ian/ghc/6.12-branch/val/dest/Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925/ghc-pkg --global-conf /Users/ian/ghc/6.12-branch/val/dest/Users/ian/ghc/6.12-branch/val/inst/lib/ghc-6.12.0.20090925/package.conf.d --force update - --global

Linking ghc-cabal with -threaded fixes the problem. I believe the problem is that when we are using a DESTDIR we get a large amount of warnings (more than a buffer full) like:

ghc-6.12.0.20090925: file AsmCodeGen.hi is missing (ignoring)

so Cabal's Distribution.Simple.Utils.rawSystemStdin:

      _ <- forkIO $ do _ <- evaluate (length err); return ()
      _ <- forkIO $ do _ <- evaluate (length out); return ()

      -- push all the input
      hPutStr inh input
      hClose inh

      -- wait for the program to terminate
      exitcode <- waitForProcess pid
      unless (exitcode == ExitSuccess) (die err)

deadlocks unless it is compiled with -threaded, as the forked evaluate threads don't get a chance to run.

The simple fix is to link with -threaded, but I'm not sure that we want to assume that -threaded works - especially as it needs to work in the bootstrapping compiler.

Can we eliminate calls to this, and any other problematic, functions?

[duncan]

That code really should not deadlock, even in the single threaded rts. It's just pushing and pulling from pipes. Is the IO library creating pipes in blocking mode or something? If so, that's what needs fixing. As far as I can see this code is correct.

[duncan]

BTW, if it is that the pipes are opened in blocking mode, it's unnecessary. The blocking mode of each end of the pipe can be set independently, so it's possible to pass a blocking mode pipe to the other process and keep our end non-blocking.

[duncan]

Ian points out that it's the waitForProcess that is the problem. That is the thing that blocks the whole process, when we're using the single threaded rts.

We conjecture this code is impossible to write correctly using runInteractiveProcess and the single threaded rts. It may be possible using the newer createProcess function.

[simonmar]

I mentioned this to Ian this morning on the phone, but just to record it here: it is indeed possible to write this code correctly using the single-threaded RTS, an example of how to do so is the implementation of readProcess and readProcessWithExitCode in recent versions of System.Process. Here's a snippet:

readProcess cmd args input = do
    (Just inh, Just outh, _, pid) <-
        createProcess (proc cmd args){ std_in  = CreatePipe,
                                       std_out = CreatePipe,
                                       std_err = Inherit }

    -- fork off a thread to start consuming the output
    output  <- hGetContents outh
    outMVar <- newEmptyMVar
    _ <- forkIO $ C.evaluate (length output) >> putMVar outMVar ()

    -- now write and flush any input
    when (not (null input)) $ do hPutStr inh input; hFlush inh
    hClose inh -- done with stdin

    -- wait on the output
    takeMVar outMVar
    hClose outh

    -- wait on the process
    ex <- waitForProcess pid

    ...

the idea is that you block on an MVar instead of in waitForProcess. This assumes that if the process goes away then the pipe will also go away and the threads will stop. Actually I think the code should be using finally to make sure the MVar gets written even if an exception is raised.

[duncan]

Ah yes, ok and I guess that extends ok to discarding both stdout and stderr. We just wait for both MVars.

Perhaps it'd be useful to have an extra StdStream constructor for UseNullHandle (or a better name) to use /dev/null on POSIX or nul on Windows.

[igloo]

Fixed:

Mon Sep 28 10:45:53 PDT 2009  Ian Lynagh <igloo@earth.li>
  * Fix deadlocks when calling waitForProcess; GHC trac #3542
  We now wait on MVars indicating that stdout and stderr have been
  closed before making the waitForProcess call.