convert to withCreateProcess for async exception safety

Not yet 100% done, so far I've grepped for waitForProcess and converted
everything that uses that to start the process with withCreateProcess.

Except for some things like P2P.IO and Assistant.TransferrerPool,
and Utility.CoProcess, that manage a pool of processes. See #2
in https://git-annex.branchable.com/todo/more_extensive_retries_to_mask_transient_failures/#comment-209f8a8c38e63fb3a704e1282cb269c7
for how those will need to be dealt with.

checkSuccessProcess, ignoreFailureProcess, and forceSuccessProcess calls waitForProcess, so
callers of them will also need to be dealt with, and have not been yet.
This commit is contained in:
Joey Hess 2020-06-03 15:48:09 -04:00
parent 1ee5919d1e
commit 92f775eba0
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
9 changed files with 68 additions and 65 deletions

View file

@ -301,15 +301,14 @@ autoEnableSpecialRemotes = do
rp <- fromRawFilePath <$> fromRepo Git.repoPath rp <- fromRawFilePath <$> fromRepo Git.repoPath
cmd <- liftIO programPath cmd <- liftIO programPath
liftIO $ withNullHandle $ \nullh -> do liftIO $ withNullHandle $ \nullh -> do
let p = proc cmd let p = (proc cmd
[ "init" [ "init"
, "--autoenable" , "--autoenable"
] ])
(Nothing, Nothing, Nothing, pid) <- createProcess $ p
{ std_out = UseHandle nullh { std_out = UseHandle nullh
, std_err = UseHandle nullh , std_err = UseHandle nullh
, std_in = UseHandle nullh , std_in = UseHandle nullh
, cwd = Just rp , cwd = Just rp
} }
void $ waitForProcess pid withCreateProcess p $ \_ _ _ pid -> void $ waitForProcess pid
remotesChanged remotesChanged

View file

@ -258,15 +258,15 @@ prepSocket socketfile sshhost sshparams = do
-- (Except there's an unlikely false positive where a forced -- (Except there's an unlikely false positive where a forced
-- ssh command exits 255.) -- ssh command exits 255.)
tryssh extraps = liftIO $ withNullHandle $ \nullh -> do tryssh extraps = liftIO $ withNullHandle $ \nullh -> do
let p = proc "ssh" $ concat let p = (proc "ssh" $ concat
[ extraps [ extraps
, toCommand sshparams , toCommand sshparams
, [fromSshHost sshhost, "true"] , [fromSshHost sshhost, "true"]
] ])
(Nothing, Nothing, Nothing, pid) <- createProcess $ p
{ std_out = UseHandle nullh { std_out = UseHandle nullh
, std_err = UseHandle nullh , std_err = UseHandle nullh
} }
withCreateProcess p $ \_ _ _ pid -> do
exitcode <- waitForProcess pid exitcode <- waitForProcess pid
return $ case exitcode of return $ case exitcode of
ExitFailure 255 -> False ExitFailure 255 -> False
@ -458,7 +458,8 @@ runSshOptions :: [String] -> String -> IO ()
runSshOptions args s = do runSshOptions args s = do
let args' = toCommand (fromSshOptionsEnv s) ++ args let args' = toCommand (fromSshOptionsEnv s) ++ args
let p = proc "ssh" args' let p = proc "ssh" args'
exitWith =<< waitForProcess . processHandle =<< createProcess p exitcode <- withCreateProcess p $ \_ _ _ pid -> waitForProcess pid
exitWith exitcode
{- When this env var is set, git-annex is being used as a ssh-askpass {- When this env var is set, git-annex is being used as a ssh-askpass
- program, and should read the password from the specified location, - program, and should read the password from the specified location,

View file

@ -220,11 +220,12 @@ openBrowser' mcmd htmlshim realurl outh errh =
hPutStrLn (fromMaybe stdout outh) $ "Launching web browser on " ++ url hPutStrLn (fromMaybe stdout outh) $ "Launching web browser on " ++ url
hFlush stdout hFlush stdout
environ <- cleanEnvironment environ <- cleanEnvironment
(_, _, _, pid) <- createProcess p let p' = p
{ env = environ { env = environ
, std_out = maybe Inherit UseHandle outh , std_out = maybe Inherit UseHandle outh
, std_err = maybe Inherit UseHandle errh , std_err = maybe Inherit UseHandle errh
} }
withCreateProcess p' $ \_ _ _ pid -> do
exitcode <- waitForProcess pid exitcode <- waitForProcess pid
unless (exitcode == ExitSuccess) $ unless (exitcode == ExitSuccess) $
hPutStrLn (fromMaybe stderr errh) "failed to start web browser" hPutStrLn (fromMaybe stderr errh) "failed to start web browser"

View file

@ -66,7 +66,7 @@ probeRepo loc baserepo = do
, Param "--check" , Param "--check"
, Param loc , Param loc
] baserepo ] baserepo
(_, _, _, pid) <- createProcess p withCreateProcess p $ \_ _ _ pid -> do
code <- waitForProcess pid code <- waitForProcess pid
return $ case code of return $ case code of
ExitSuccess -> Decryptable ExitSuccess -> Decryptable

View file

@ -50,9 +50,11 @@ data HistoryCommit = HistoryCommit
{- Gets a History starting with the provided commit, and down to the {- Gets a History starting with the provided commit, and down to the
- requested depth. -} - requested depth. -}
getHistoryToDepth :: Integer -> Ref -> Repo -> IO (Maybe (History HistoryCommit)) getHistoryToDepth :: Integer -> Ref -> Repo -> IO (Maybe (History HistoryCommit))
getHistoryToDepth n commit r = do getHistoryToDepth n commit r = withCreateProcess p go
(_, Just inh, _, pid) <- createProcess (gitCreateProcess params r) where
p = (gitCreateProcess params r)
{ std_out = CreatePipe } { std_out = CreatePipe }
go _ (Just inh) _ pid = do
!h <- fmap (truncateHistoryToDepth n) !h <- fmap (truncateHistoryToDepth n)
. build Nothing . build Nothing
. map parsehistorycommit . map parsehistorycommit
@ -62,7 +64,8 @@ getHistoryToDepth n commit r = do
hClose inh hClose inh
void $ waitForProcess pid void $ waitForProcess pid
return h return h
where go _ _ _ _ = error "internal"
build h [] = fmap (mapHistory fst) h build h [] = fmap (mapHistory fst) h
build _ (Nothing:_) = Nothing build _ (Nothing:_) = Nothing
build Nothing (Just v:rest) = build Nothing (Just v:rest) =

View file

@ -37,14 +37,15 @@ transportUsingCmd cmd params rr@(RemoteRepo r gc) url h@(TransportHandle (LocalR
transportUsingCmd' :: FilePath -> [CommandParam] -> Transport transportUsingCmd' :: FilePath -> [CommandParam] -> Transport
transportUsingCmd' cmd params (RemoteRepo r gc) url transporthandle ichan ochan = transportUsingCmd' cmd params (RemoteRepo r gc) url transporthandle ichan ochan =
robustConnection 1 $ do robustConnection 1 $ withCreateProcess p go
(Just toh, Just fromh, Just errh, pid) <- where
createProcess (proc cmd (toCommand params)) p = (proc cmd (toCommand params))
{ std_in = CreatePipe { std_in = CreatePipe
, std_out = CreatePipe , std_out = CreatePipe
, std_err = CreatePipe , std_err = CreatePipe
} }
go (Just toh) (Just fromh) (Just errh) pid = do
-- Run all threads until one finishes and get the status -- Run all threads until one finishes and get the status
-- of the first to finish. Cancel the rest. -- of the first to finish. Cancel the rest.
status <- catchDefaultIO (Right ConnectionClosed) $ status <- catchDefaultIO (Right ConnectionClosed) $
@ -58,7 +59,8 @@ transportUsingCmd' cmd params (RemoteRepo r gc) url transporthandle ichan ochan
void $ waitForProcess pid void $ waitForProcess pid
return $ either (either id id) id status return $ either (either id id) id status
where go _ _ _ _ = error "internal"
send msg = atomically $ writeTChan ochan msg send msg = atomically $ writeTChan ochan msg
fetch = do fetch = do

View file

@ -135,8 +135,8 @@ runner opts
pp <- Annex.Path.programPath pp <- Annex.Path.programPath
Utility.Env.Set.setEnv subenv "1" True Utility.Env.Set.setEnv subenv "1" True
ps <- getArgs ps <- getArgs
(Nothing, Nothing, Nothing, pid) <- createProcess (proc pp ps) exitcode <- withCreateProcess (proc pp ps) $
exitcode <- waitForProcess pid \_ _ _ pid -> waitForProcess pid
unless (keepFailuresOption opts) finalCleanup unless (keepFailuresOption opts) finalCleanup
exitWith exitcode exitWith exitcode
runsubprocesstests (Just _) = isolateGitConfig $ do runsubprocesstests (Just _) = isolateGitConfig $ do

View file

@ -458,9 +458,9 @@ setTestMode testmode = do
] ]
runFakeSsh :: [String] -> IO () runFakeSsh :: [String] -> IO ()
runFakeSsh ("-n":ps) = runFakeSsh ps runFakeSsh ("-n":ps) = runFakeSsh ps
runFakeSsh (_host:cmd:[]) = do runFakeSsh (_host:cmd:[]) =
(_, _, _, pid) <- createProcess (shell cmd) withCreateProcess (shell cmd) $
exitWith =<< waitForProcess pid \_ _ _ pid -> exitWith =<< waitForProcess pid
runFakeSsh ps = error $ "fake ssh option parse error: " ++ show ps runFakeSsh ps = error $ "fake ssh option parse error: " ++ show ps
getTestMode :: IO TestMode getTestMode :: IO TestMode

View file

@ -50,24 +50,21 @@ processTranscript'' cp input = do
System.Posix.IO.setFdOption writef System.Posix.IO.CloseOnExec True System.Posix.IO.setFdOption writef System.Posix.IO.CloseOnExec True
readh <- System.Posix.IO.fdToHandle readf readh <- System.Posix.IO.fdToHandle readf
writeh <- System.Posix.IO.fdToHandle writef writeh <- System.Posix.IO.fdToHandle writef
p@(_, _, _, pid) <- createProcess $ cp withCreateProcess cp $ \hin hout herr pid -> do
{ std_in = if isJust input then CreatePipe else Inherit
, std_out = UseHandle writeh
, std_err = UseHandle writeh
}
hClose writeh hClose writeh
get <- asyncreader readh get <- asyncreader readh
writeinput input p writeinput input (hin, hout, herr, pid)
transcript <- wait get transcript <- wait get
#else #else
{- This implementation for Windows puts stderr after stdout. -} {- This implementation for Windows puts stderr after stdout. -}
p@(_, _, _, pid) <- createProcess $ cp let cp' = cp
{ std_in = if isJust input then CreatePipe else Inherit { std_in = if isJust input then CreatePipe else Inherit
, std_out = CreatePipe , std_out = CreatePipe
, std_err = CreatePipe , std_err = CreatePipe
} }
withCreateProcess cp' \hin hout herr pid -> do
let p = (hin, hout, herr, pid)
getout <- asyncreader (stdoutHandle p) getout <- asyncreader (stdoutHandle p)
geterr <- asyncreader (stderrHandle p) geterr <- asyncreader (stderrHandle p)
writeinput input p writeinput input p