From 1b2f29b2079c80089550c13d4d7f040169084400 Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Mon, 13 Jul 2020 12:16:48 -0400
Subject: [PATCH] comment

---
 ..._5274e7f715ef060383046e8f5da08164._comment | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 doc/bugs/Recent_hang_with_rsync_remote_with_older_systems___40__Xenial__44___Jessie__41__/comment_1_5274e7f715ef060383046e8f5da08164._comment

diff --git a/doc/bugs/Recent_hang_with_rsync_remote_with_older_systems___40__Xenial__44___Jessie__41__/comment_1_5274e7f715ef060383046e8f5da08164._comment b/doc/bugs/Recent_hang_with_rsync_remote_with_older_systems___40__Xenial__44___Jessie__41__/comment_1_5274e7f715ef060383046e8f5da08164._comment
new file mode 100644
index 0000000000..40807725de
--- /dev/null
+++ b/doc/bugs/Recent_hang_with_rsync_remote_with_older_systems___40__Xenial__44___Jessie__41__/comment_1_5274e7f715ef060383046e8f5da08164._comment
@@ -0,0 +1,58 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2020-07-13T15:53:56Z"
+ content="""
+Hmm, my guess is that the significant and unmentioned change in that commit
+is, it waits for one of the stdout or stderr to be fully processed before
+waiting on the process to terminate. So if it somehow fails to detect EOF
+(maybe because old rsync and ssh somehow end up with the handle not being
+closed when rsync exits eg if a background ssh process inherited them?) it
+would block forever.
+
+If so, this patch would avoid the hang (and if this doesn't fix it, the
+problem must be somehow due to using with CreateProcess).
+
+	diff --git a/Utility/Metered.hs b/Utility/Metered.hs
+	index 1c35a9a05..8e363681d 100644
+	--- a/Utility/Metered.hs
+	+++ b/Utility/Metered.hs
+	@@ -314,9 +314,8 @@ outputFilter cmd params environ outfilter errfilter =
+	 	catchMaybeIO $ withCreateProcess p go
+	   where
+	 	go _ (Just outh) (Just errh) pid = do
+	-		void $ concurrently 
+	-			(tryIO (outfilter outh) >> hClose outh)
+	-			(tryIO (errfilter errh) >> hClose errh)
+	+		void $ async $ tryIO (outfilter outh) >> hClose outh
+	+		void $ async $ tryIO (errfilter errh) >> hClose errh
+	 		waitForProcess pid
+	 	go _ _ _ _ = error "internal"
+ 
+If that patch does avoid the hang, this one would be worth a try
+as it tries to accomplish the goal of the patch in a better way:
+
+	diff --git a/Utility/Metered.hs b/Utility/Metered.hs
+	index 1c35a9a05..bd265ae09 100644
+	--- a/Utility/Metered.hs
+	+++ b/Utility/Metered.hs
+	@@ -313,11 +313,10 @@ outputFilter
+	 outputFilter cmd params environ outfilter errfilter = 
+	 	catchMaybeIO $ withCreateProcess p go
+	   where
+	-	go _ (Just outh) (Just errh) pid = do
+	-		void $ concurrently 
+	-			(tryIO (outfilter outh) >> hClose outh)
+	-			(tryIO (errfilter errh) >> hClose errh)
+	-		waitForProcess pid
+	+	go _ (Just outh) (Just errh) pid =
+	+		withAsync (tryIO (outfilter outh) >> hClose outh) $ const $
+	+			withAsync (tryIO (errfilter errh) >> hClose errh) $ const $
+	+				waitForProcess pid
+	 	go _ _ _ _ = error "internal"
+	 	
+	 	p = (proc cmd (toCommand params))
+
+I can try these if necessary but don't have docker handy,
+so maybe you're in a better position to?
+"""]]