From d2af6baaebedcd22830a99957e23383b33bb364f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 15 Mar 2018 16:14:22 -0400 Subject: [PATCH] fixed processTranscript hang problem The pipe's FDs got inherited by ssh and it did something that kept them open even once it exited. Probably involving passing them on to the ssh mux daemon. Set close on exec, and all is well. Kept Annex.Ssh not using processTranscript even though it no longer hangs when it does use it, just because processTranscript is overkill there. This commit was supported by the NSF-funded DataLad project. --- Utility/Process/Transcript.hs | 2 ++ doc/bugs/occasional_hang_with_p2pstdio.mdwn | 24 +++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Utility/Process/Transcript.hs b/Utility/Process/Transcript.hs index 88a0a77638..68fb2223eb 100644 --- a/Utility/Process/Transcript.hs +++ b/Utility/Process/Transcript.hs @@ -42,6 +42,8 @@ processTranscript'' cp input = do {- This implementation interleves stdout and stderr in exactly the order - the process writes them. -} (readf, writef) <- System.Posix.IO.createPipe + System.Posix.IO.setFdOption readf System.Posix.IO.CloseOnExec True + System.Posix.IO.setFdOption writef System.Posix.IO.CloseOnExec True readh <- System.Posix.IO.fdToHandle readf writeh <- System.Posix.IO.fdToHandle writef p@(_, _, _, pid) <- createProcess $ cp diff --git a/doc/bugs/occasional_hang_with_p2pstdio.mdwn b/doc/bugs/occasional_hang_with_p2pstdio.mdwn index a46e59c35d..ed7c4decd7 100644 --- a/doc/bugs/occasional_hang_with_p2pstdio.mdwn +++ b/doc/bugs/occasional_hang_with_p2pstdio.mdwn @@ -41,14 +41,26 @@ code. > (Arguably there should be locking to prevent multiple threads doing > this, but the actual overhead of multiple threads doing it may be > smaller than the overhead of such added locking.) +> +> Converted makeconnection to not use processTranscript, +> and that does seem to avoid the hang. > > Why is makeconnection's use of processTranscript hanging? > processTranscriot tries to read the process's output (ssh has none), > and waiting for the output to get read is for some reason hanging -> forever, despite the ssh process becoming a zombie. -> Converted makeconnection to not use processTranscript, -> and that does seem to avoid the hang. +> forever, despite the ssh process becoming a zombie. I actually +> rewrote the part of processtranscript that reads the process's input, +> to be a much simpler use of async, and that new implementation has the +> same problem. It's hanging, not throwing an exception. Most puzzling! +> +> Hmm.. Perhaps the write handle is staying open even after ssh exits? +> processTranscript sets up a pipe for the process to write to, and +> the ssh process may inherit the FDs for that pipe (other than as stdout and +> stderr). If the write handle +> remains open, reading from it would block. Since the ssh mux server is +> involved, and the handle might be passed to it or something, that seems +> at least possible as the cause. The windows version of createProcess +> does not use that pipe, and switching to use it does avoid the hang. +> Yep! Setting the pipe's FDs to close on exec did avoid the hang. > -> So, this bug is left open only because processTranscript hangs in situations -> like this. No other uses of it involve concurrency, but we still need to -> get to the bottom of its hang.. --[[Joey]] +> [[done]] --[[Joey]]