From 15d617f7e117daf7647d35735c1dc10b6803f78f Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Fri, 19 Nov 2021 11:53:25 -0400
Subject: [PATCH] have setConcurrency stop any running git coprocesses

When non-concurrent git coprocesses have been started, setConcurrency
used to not stop them, and so could leak processes when enabling
concurrency, eg when forkState is called.

I do not think that ever actually happened, given where setConcurrency
is called. And it probably would only leak one of each process, since it
never downgrades from concurrent to non-concurrent.
---
 Annex/Concurrent.hs                           | 38 +++++++++++++------
 Types/Concurrency.hs                          |  1 +
 ..._b48f8d801305d9bde08060f06634b88e._comment | 12 ++++++
 3 files changed, 39 insertions(+), 12 deletions(-)
 create mode 100644 doc/forum/git-annex_causing_zombie_git_processes_to_build_up/comment_12_b48f8d801305d9bde08060f06634b88e._comment

diff --git a/Annex/Concurrent.hs b/Annex/Concurrent.hs
index fe8b7a8d3e..0851df8c91 100644
--- a/Annex/Concurrent.hs
+++ b/Annex/Concurrent.hs
@@ -17,6 +17,7 @@ import qualified Annex.Queue
 import Annex.Action
 import Types.Concurrency
 import Types.CatFileHandles
+import Annex.CatFile
 import Annex.CheckAttr
 import Annex.CheckIgnore
 
@@ -32,18 +33,31 @@ setConcurrency' NonConcurrent f =
 		{ Annex.concurrency = f NonConcurrent
 		}
 setConcurrency' c f = do
-	cfh <- getState Annex.catfilehandles
-	cfh' <- case cfh of
-		CatFileHandlesNonConcurrent _ -> liftIO catFileHandlesPool
-		CatFileHandlesPool _ -> pure cfh
-	cah <- mkConcurrentCheckAttrHandle c
-	cih <- mkConcurrentCheckIgnoreHandle c
-	Annex.changeState $ \s -> s
-		{ Annex.concurrency = f c
-		, Annex.catfilehandles = cfh'
-		, Annex.checkattrhandle = Just cah
-		, Annex.checkignorehandle = Just cih
-		}
+	oldc <- Annex.getState Annex.concurrency
+	case oldc of
+		ConcurrencyCmdLine NonConcurrent -> fromnonconcurrent
+		ConcurrencyGitConfig NonConcurrent -> fromnonconcurrent
+		_
+			| oldc == newc -> return ()
+			| otherwise ->
+				Annex.changeState $ \s -> s
+					{ Annex.concurrency = newc
+					}
+  where
+	newc = f c
+	fromnonconcurrent = do
+		catFileStop
+		checkAttrStop
+		checkIgnoreStop
+		cfh <- liftIO catFileHandlesPool
+		cah <- mkConcurrentCheckAttrHandle c
+		cih <- mkConcurrentCheckIgnoreHandle c
+		Annex.changeState $ \s -> s
+			{ Annex.concurrency = newc
+			, Annex.catfilehandles = cfh
+			, Annex.checkattrhandle = Just cah
+			, Annex.checkignorehandle = Just cih
+			}
 
 {- Allows forking off a thread that uses a copy of the current AnnexState
  - to run an Annex action.
diff --git a/Types/Concurrency.hs b/Types/Concurrency.hs
index ccab1ee0a9..9204f1d0f4 100644
--- a/Types/Concurrency.hs
+++ b/Types/Concurrency.hs
@@ -22,3 +22,4 @@ parseConcurrency s = Concurrent <$> readish s
 data ConcurrencySetting
 	= ConcurrencyCmdLine Concurrency
 	| ConcurrencyGitConfig Concurrency
+	deriving (Eq)
diff --git a/doc/forum/git-annex_causing_zombie_git_processes_to_build_up/comment_12_b48f8d801305d9bde08060f06634b88e._comment b/doc/forum/git-annex_causing_zombie_git_processes_to_build_up/comment_12_b48f8d801305d9bde08060f06634b88e._comment
new file mode 100644
index 0000000000..c2df933601
--- /dev/null
+++ b/doc/forum/git-annex_causing_zombie_git_processes_to_build_up/comment_12_b48f8d801305d9bde08060f06634b88e._comment
@@ -0,0 +1,12 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 12"""
+ date="2021-11-19T15:37:43Z"
+ content="""
+I found a case where zombie git processes could be started in theory, 
+but only when git-annex is run without -J. And only a few zombies I think.
+And I couldn't find a code path where it actually happened.
+So not the same as this bug. But it did involve setConcurrency, which 
+the bisected commit also involves (via forkState), so at least shows
+how that could cause a such a problem in theory. Fixed that.
+"""]]