Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that waitForProcess is never called more than once (fixes #69) #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# ChangeLog for typed-process

## 0.2.12.0

* Ensure that `waitForProcess` is never called more than once
[#70](https://github.com/fpco/typed-process/pull/70)

## 0.2.11.0

* Expose more from `System.Process.Typed.Internal`
Expand Down
2 changes: 1 addition & 1 deletion package.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: typed-process
version: 0.2.11.0
version: 0.2.12.0
synopsis: Run external processes, with strong typing of streams
description: Please see the tutorial at <https://github.com/fpco/typed-process#readme>
category: System
Expand Down
28 changes: 11 additions & 17 deletions src/System/Process/Typed.hs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE TypeFamilies #-}
{-# LANGUAGE DeriveDataTypeable #-}
{-# LANGUAGE RecordWildCards #-}
Expand Down Expand Up @@ -130,13 +131,15 @@ module System.Process.Typed
) where

import Control.Exception hiding (bracket, finally)
import Control.Monad ((>=>), guard)
import Control.Monad.IO.Class
import qualified System.Process as P
import System.IO (hClose)
import System.IO.Error (isPermissionError)
import Control.Concurrent (threadDelay)
import Control.Concurrent.Async (asyncWithUnmask, cancel, waitCatch)
import Control.Concurrent.STM (newEmptyTMVarIO, atomically, putTMVar, TMVar, readTMVar, tryReadTMVar, STM, tryPutTMVar, throwSTM, catchSTM)
import Control.Concurrent.Async (asyncWithUnmask)
import qualified Control.Concurrent.Async as Async
import Control.Concurrent.STM (newEmptyTMVarIO, atomically, putTMVar, TMVar, readTMVar, tryReadTMVar, STM, throwSTM, catchSTM)
import System.Exit (ExitCode (ExitSuccess, ExitFailure))
import System.Process.Typed.Internal
import qualified Data.ByteString.Lazy as L
Expand Down Expand Up @@ -239,27 +242,18 @@ startProcess pConfig'@ProcessConfig {..} = liftIO $ do
atomically $ putTMVar pExitCode ec
return ec

let waitForProcess = Async.wait waitingThread :: IO ExitCode
let pCleanup = pCleanup1 `finally` do
-- First: stop calling waitForProcess, so that we can
-- avoid race conditions where the process is removed from
-- the system process table while we're trying to
-- terminate it.
cancel waitingThread

-- Now check if the process had already exited
eec <- waitCatch waitingThread

case eec of
_ :: ExitCode <- Async.poll waitingThread >>= \ case
-- Process already exited, nothing to do
Right _ec -> return ()
Just r -> either throwIO return r
Copy link
Contributor Author

@sol sol May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time this will be a Right-value, but with delegate_ctlc this can be UserInterrupt, which we then re-throw here (see #73).


-- Process didn't exit yet, let's terminate it and
-- then call waitForProcess ourselves
Left _ -> do
Nothing -> do
terminateProcess pHandle
ec <- P.waitForProcess pHandle
success <- atomically $ tryPutTMVar pExitCode ec
evaluate $ assert success ()
waitForProcess
return ()

return Process {..}
where
Expand Down
10 changes: 10 additions & 0 deletions test/System/Process/TypedSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module System.Process.TypedSpec (spec) where
import System.Process.Typed
import System.Process.Typed.Internal
import System.IO
import Control.Exception
import Control.Concurrent.Async (Concurrently (..))
import Control.Concurrent.STM (atomically)
import Test.Hspec
Expand Down Expand Up @@ -170,3 +171,12 @@ spec = do
it "empty param are showed" $
let expected = "Raw command: podman exec --detach-keys \"\" ctx bash\n"
in show (proc "podman" ["exec", "--detach-keys", "", "ctx", "bash"]) `shouldBe` expected

describe "stopProcess" $ do
it "never calls waitForProcess more than once (fix for #69)" $ do
-- https://github.com/fpco/typed-process/issues/70
let config = setStdout createPipe (proc "echo" ["foo"])
withProcessWait config $ \p -> do
_ <- S.hGetContents (getStdout p)
throwIO DivideByZero
`shouldThrow` (== DivideByZero)