have setConcurrency stop any running git coprocesses
authorJoey Hess <joeyh@joeyh.name>
Fri, 19 Nov 2021 15:53:25 +0000 (11:53 -0400)
committerJoey Hess <joeyh@joeyh.name>
Fri, 19 Nov 2021 16:00:39 +0000 (12:00 -0400)
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
Types/Concurrency.hs
doc/forum/git-annex_causing_zombie_git_processes_to_build_up/comment_12_b48f8d801305d9bde08060f06634b88e._comment [new file with mode: 0644]

index fe8b7a8d3e2a984e24b4e0cd46871abcab635ca6..0851df8c912b2975051b2118242711e68acd7c56 100644 (file)
@@ -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.
index ccab1ee0a910d413a566becdec3ffa64a03fc9e5..9204f1d0f41b7adb5f6c3687eb8453d19d7f87bb 100644 (file)
@@ -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 (file)
index 0000000..c2df933
--- /dev/null
@@ -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.
+"""]]