Improve shutdown process for external special remotes and external backends

Make sure to relay any remaining stderr from the process after it has
shut down, rather than closing stderr just before shutdown. This avoids
a situation where the process is still running and tries to write to
stderr, getting a SIGPIPE. And, it ensures that no stderr output is
lost.

This may fix a problem encountered by datalad on windows, where it hangs
during the external special remote shutdown.

Before commit a49d300545, it closed stdin
and stdout, but left stderr open, and never killed the stderr waiter
thread, which presumably exited on its own. For async exception
safety, do need to at make sure that thread gets waited on, as that
commit does, but it introduced this problem.

Note that, the process's stdout is closed before waiting on it. It's too
late for anything it writes to stdout to be processed, and since we're
not going to consume any such writes, this avoids the process getting
blocked writing to stdout due to us not reading what it's buffered. This
does mean that if the process writes to stdout too late, it will get a
SIGPIPE. (This was already the case before the above-mentioned commit.)
In practice, I think only the protocol's ERROR is allowed to be
sent at a point where this could happen.
This commit is contained in:
Joey Hess 2020-11-02 12:35:07 -04:00
parent 59dbd10f92
commit 7245a9ed53
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
2 changed files with 16 additions and 6 deletions

View file

@ -55,14 +55,20 @@ startExternalAddonProcess basecmd pid = do
started cmd errrelayer pall@(Just hin, Just hout, Just herr, ph) = do
stderrelay <- async $ errrelayer herr
let shutdown forcestop = do
cancel stderrelay
-- Close the process's stdin, to let it know there
-- are no more requests, so it will exit.
hClose hout
-- Close the procces's stdout as we're not going to
-- process any more output from it.
hClose hin
if forcestop
then cleanupProcess pall
else flip onException (cleanupProcess pall) $ do
else void (waitForProcess ph)
`onException` cleanupProcess pall
-- This thread will exit after consuming any
-- remaining stderr from the process.
wait stderrelay
hClose herr
hClose hin
hClose hout
void $ waitForProcess ph
return $ ExternalAddonProcess
{ externalSend = hin
, externalReceive = hout

View file

@ -23,6 +23,10 @@ git-annex (8.20201008) UNRELEASED; urgency=medium
on this.
* view: Avoid using ':' from metadata when generating a view, because
it's a special character on Windows ("c:")
* Improve shutdown process for external special remotes and external
backends. Make sure to relay any remaining stderr from the process,
and potentially avoid the process getting a SIGPIPE if it writes to
stderr too late.
-- Joey Hess <id@joeyh.name> Thu, 08 Oct 2020 10:48:17 -0400