Skip to content

Commit

Permalink
Use jobs when calling subprocesses
Browse files Browse the repository at this point in the history
Many toolchain tools written for POSIX systems rely on the exec system
call. Unfortunately, it is not possible to implement `exec` in a
POSIX-compliant manner on Windows. In particular, the semantics of the
`exec` implementation provided by the widely-used `msvcrt` C runtime
will cause process's waiting on the `exec`'ing process to incorrectly
conclude that the process has successfully terminated when in fact it is
still running in another process.

For this reason, the `process` library exposes the `use_process_jobs`
flag to use a more strict (although still not POSIX-compliant) mechanism
for tracking process completion. This is explained in this comment [2].

Unfortunately, job support in the `process` library is currently quite
broken and was only recently fixed [1]. Consequently, we only enable job
object support for process releases >= 1.6.8.

[1] haskell/process#168
[2] https://github.com/haskell/process/blob/master/System/Process.hs#L399
  • Loading branch information
bgamari committed Feb 6, 2020
1 parent b925356 commit ab70010
Showing 1 changed file with 30 additions and 4 deletions.
34 changes: 30 additions & 4 deletions Cabal/Distribution/Simple/Utils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ import Numeric (showFFloat)
import qualified System.Process as Process
( CreateProcess(..), StdStream(..), proc)
import System.Process
( ProcessHandle, createProcess, rawSystem, runInteractiveProcess
( ProcessHandle, createProcess, rawSystem
, showCommandForUser, waitForProcess)

import qualified GHC.IO.Exception as GHC
Expand Down Expand Up @@ -680,6 +680,24 @@ maybeExit cmd = do
res <- cmd
unless (res == ExitSuccess) $ exitWith res

-- | Enable process jobs to ensure accurate determination of process completion
-- in the presence of @exec(3)@ on Windows.
--
-- Unfortunately the process job support is badly broken in @process@ releases
-- prior to 1.6.8, so we disable it in these versions, despite the fact that
-- this means we may see sporatic build failures without jobs.
enableProcessJobs :: CreateProces -> CreateProcess
#ifdef MIN_VERSION_process
#if MIN_VERSION_process(1,6,8)
enableProcessJobs cp = cp {Process.use_process_jobs = True}
#else
enableProcessJobs cp = cp
#endif
#else
enableProcessJobs cp = cp
#endif


printRawCommandAndArgs :: Verbosity -> FilePath -> [String] -> IO ()
printRawCommandAndArgs verbosity path args = withFrozenCallStack $
printRawCommandAndArgsAndEnv verbosity path args Nothing Nothing
Expand Down Expand Up @@ -726,7 +744,7 @@ rawSystemExitWithEnv :: Verbosity
rawSystemExitWithEnv verbosity path args env = withFrozenCallStack $ do
printRawCommandAndArgsAndEnv verbosity path args Nothing (Just env)
hFlush stdout
(_,_,_,ph) <- createProcess $
(_,_,_,ph) <- createProcess $ enableProcessJobs
(Process.proc path args) { Process.env = (Just env)
#ifdef MIN_VERSION_process
#if MIN_VERSION_process(1,2,0)
Expand Down Expand Up @@ -777,7 +795,7 @@ createProcessWithEnv ::
createProcessWithEnv verbosity path args mcwd menv inp out err = withFrozenCallStack $ do
printRawCommandAndArgsAndEnv verbosity path args mcwd menv
hFlush stdout
(inp', out', err', ph) <- createProcess $
(inp', out', err', ph) <- createProcess $ enableProcessJobs
(Process.proc path args) {
Process.cwd = mcwd
, Process.env = menv
Expand Down Expand Up @@ -823,7 +841,15 @@ rawSystemStdInOut verbosity path args mcwd menv input _ = withFrozenCallStack $
printRawCommandAndArgs verbosity path args

Exception.bracket
(runInteractiveProcess path args mcwd menv)
(do (Just inh,Just outh, Just errh,pid) <-
createProcess $ enableProcessJobs
(Process.proc path args) { Process.cwd = mcwd
, Process.env = menv
, Process.std_in = Process.CreatePipe
, Process.std_out = Process.CreatePipe
, Process.std_err = Process.CreatePipe
}
return (inh, outh, errh, pid))
(\(inh,outh,errh,_) -> hClose inh >> hClose outh >> hClose errh)
$ \(inh,outh,errh,pid) -> do

Expand Down

0 comments on commit ab70010

Please sign in to comment.