Skip to content

Commit c86522c

Browse files
committed
Fix cleanup when stdin/stdout handles are inherited and are Nothing
1 parent c1ce40e commit c86522c

File tree

1 file changed

+52
-15
lines changed

1 file changed

+52
-15
lines changed

src/Streamly/Internal/System/Process.hs

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ module Streamly.Internal.System.Process
7777
, inheritStdin
7878
, inheritStdout
7979
, pipeStdErr
80+
, pipeStdin
81+
, pipeStdout
8082

8183
-- * Exceptions
8284
, ProcessFailure (..)
@@ -214,6 +216,12 @@ mkConfig _ _ = Config False
214216
pipeStdErr :: Config -> Config
215217
pipeStdErr (Config _) = Config True
216218

219+
pipeStdin :: Config -> Config
220+
pipeStdin (Config _) = Config True
221+
222+
pipeStdout :: Config -> Config
223+
pipeStdout (Config _) = Config True
224+
217225
inheritStdin :: Config -> Config
218226
inheritStdin (Config _) = Config True
219227

@@ -453,6 +461,12 @@ waitForChildTree = waitForDescendants
453461
pipeStdErr :: Config -> Config
454462
pipeStdErr (Config cfg) = Config $ cfg { std_err = CreatePipe }
455463

464+
pipeStdin :: Config -> Config
465+
pipeStdin (Config cleanupCfg cfg) = Config cleanupCfg $ cfg { std_in = CreatePipe }
466+
467+
pipeStdout :: Config -> Config
468+
pipeStdout (Config cleanupCfg cfg) = Config cleanupCfg $ cfg { std_out = CreatePipe }
469+
456470
inheritStdin :: Config -> Config
457471
inheritStdin (Config cfg) = Config $ cfg { std_in = Inherit }
458472

@@ -518,7 +532,7 @@ cleanupNormal (_, _, _, procHandle) = do
518532
-- still hanging around.
519533
cleanupException ::
520534
(Maybe Handle, Maybe Handle, Maybe Handle, ProcessHandle) -> IO ()
521-
cleanupException (Just stdinH, Just stdoutH, stderrMaybe, ph) = do
535+
cleanupException (stdinMaybe, stdoutMaybe, stderrMaybe, ph) = do
522536
-- Send a SIGTERM to the process
523537
#ifdef USE_NATIVE
524538
terminate ph
@@ -529,8 +543,8 @@ cleanupException (Just stdinH, Just stdoutH, stderrMaybe, ph) = do
529543
-- Ideally we should be closing the handle without flushing the buffers so
530544
-- that we cannot get a SIGPIPE. But there seems to be no way to do that as
531545
-- of now so we just ignore the SIGPIPE.
532-
hClose stdinH `catch` eatSIGPIPE
533-
hClose stdoutH
546+
whenJust (\stdinH -> hClose stdinH `catch` eatSIGPIPE) stdinMaybe
547+
whenJust hClose stdoutMaybe
534548
whenJust hClose stderrMaybe
535549

536550
-- Non-blocking wait for the process to go away
@@ -553,7 +567,6 @@ cleanupException (Just stdinH, Just stdoutH, stderrMaybe, ph) = do
553567
_ -> False
554568

555569
eatSIGPIPE e = unless (isSIGPIPE e) $ throwIO e
556-
cleanupException _ = error "cleanupProcess: Not reachable"
557570

558571
-- | Creates a system process from an executable path and arguments. For the
559572
-- default attributes used to create the process see 'mkConfig'.
@@ -618,6 +631,11 @@ pipeChunksWithAction run modCfg path args =
618631

619632
alloc = createProc' modCfg path args
620633

634+
-- Note: It is allowed to inheritStdout or inheritStderr but not both as that
635+
-- would not generate a stream for further processing.
636+
-- inheritStdin has no affect, we always pipe the input stream to the process's
637+
-- stdin
638+
621639
-- | Like 'pipeChunksEither' but use the specified configuration to run the
622640
-- process.
623641
{-# INLINE pipeChunksEitherWith #-}
@@ -629,10 +647,12 @@ pipeChunksEitherWith ::
629647
-> Stream m (Array Word8) -- ^ Input stream
630648
-> Stream m (Either (Array Word8) (Array Word8)) -- ^ Output stream
631649
pipeChunksEitherWith modifier path args input =
632-
pipeChunksWithAction run (modifier . pipeStdErr) path args
650+
pipeChunksWithAction run (pipeStdin . modifier . pipeStdErr) path args
633651

634652
where
635653

654+
run (_, Nothing, Nothing, _) =
655+
error "pipeChunksEitherWith: only one of stdout or stderr can be inherited"
636656
run (Just stdinH, Just stdoutH, Just stderrH, _) =
637657
putChunksClose stdinH input
638658
`parallel` fmap Left (toChunksClose stderrH)
@@ -692,6 +712,10 @@ pipeBytesEither path args input =
692712
rightRdr = fmap Right Array.reader
693713
in Stream.unfoldMany (Unfold.either leftRdr rightRdr) output
694714

715+
-- Note: inheritStdin, inheritStdout have no affect, we always pipe
716+
-- the input stream to the process's stdin and pipe the stdout to the
717+
-- resulting stream
718+
695719
-- | Like 'pipeChunks' but use the specified configuration to run the process.
696720
{-# INLINE pipeChunksWith #-}
697721
pipeChunksWith ::
@@ -702,13 +726,14 @@ pipeChunksWith ::
702726
-> Stream m (Array Word8) -- ^ Input stream
703727
-> Stream m (Array Word8) -- ^ Output stream
704728
pipeChunksWith modifier path args input =
705-
pipeChunksWithAction run modifier path args
729+
pipeChunksWithAction run (pipeStdout . pipeStdin . modifier) path args
706730

707731
where
708732

709733
run (Just stdinH, Just stdoutH, _, _) =
710-
putChunksClose stdinH input `parallel` toChunksClose stdoutH
711-
run _ = error "pipeChunksWith: Not reachable"
734+
putChunksClose stdinH input
735+
`parallel` toChunksClose stdoutH
736+
run _ = error "pipeChunksWith: unreachable"
712737

713738
-- | @pipeChunks file args input@ runs the executable @file@ specified by
714739
-- its name or path using @args@ as arguments and @input@ stream as its
@@ -841,6 +866,10 @@ pipeChars path args input =
841866
-- Generation
842867
-------------------------------------------------------------------------------
843868

869+
-- Note: It is allowed to inheritStdout or inheritStderr but not both as that
870+
-- would not generate a stream for further processing and would result in unintuitive
871+
-- behaviour
872+
844873
-- | Like 'toChunksEither' but use the specified configuration to run the
845874
-- process.
846875
{-# INLINE toChunksEitherWith #-}
@@ -851,14 +880,18 @@ toChunksEitherWith ::
851880
-> [String] -- ^ Arguments
852881
-> Stream m (Either (Array Word8) (Array Word8)) -- ^ Output stream
853882
toChunksEitherWith modifier path args =
854-
pipeChunksWithAction run (modifier . inheritStdin . pipeStdErr) path args
883+
pipeChunksWithAction run (modifier . inheritStdin . pipeStdErr . pipeStdout) path args
855884

856885
where
857886

858-
run (_, Just stdoutH, Just stderrH, _) =
859-
fmap Left (toChunksClose stderrH)
860-
`parallel` fmap Right (toChunksClose stdoutH)
861-
run _ = error "toChunksEitherWith: Not reachable"
887+
run (_, Nothing, Nothing, _) =
888+
error "toChunksEitherWith: only one of stdout or stderr can be inherited"
889+
run (_, stdoutMaybe, stderrMaybe, _) =
890+
fmap Left (whenJustS toChunksClose stderrMaybe)
891+
`parallel` fmap Right (whenJustS toChunksClose stdoutMaybe)
892+
893+
-- Note: inheritStdout has no affect, we always pipe stdout to the resulting
894+
-- stream
862895

863896
-- | Like 'toChunks' but use the specified configuration to run the process.
864897
{-# INLINE toChunksWith #-}
@@ -869,12 +902,12 @@ toChunksWith ::
869902
-> [String] -- ^ Arguments
870903
-> Stream m (Array Word8) -- ^ Output stream
871904
toChunksWith modifier path args =
872-
pipeChunksWithAction run (modifier . inheritStdin) path args
905+
pipeChunksWithAction run (pipeStdout . modifier . inheritStdin) path args
873906

874907
where
875908

876909
run (_, Just stdoutH, _, _) = toChunksClose stdoutH
877-
run _ = error "toChunksWith: Not reachable"
910+
run _ = error "toChunksWith: Not reachable"
878911

879912
-- | @toBytesEither path args@ runs the executable at @path@ using @args@ as
880913
-- arguments and returns the output of the process as a stream of 'Either'
@@ -1181,3 +1214,7 @@ daemon modCfg path args =
11811214
(setSession NewSession . modCfg)
11821215
path args
11831216
in fmap (either undefined id) r
1217+
1218+
1219+
whenJustS :: Applicative m => (a -> Stream m b) -> Maybe a -> Stream m b
1220+
whenJustS action mb = maybe Stream.nil action mb

0 commit comments

Comments
 (0)