From adf5195082c9d70fc6b7f677417b0cc93a1a33eb Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 17 Sep 2012 21:32:30 -0400 Subject: [PATCH] run current branch merge in annex monad I was seeing some interesting crashes after the previous commit, when making file changes slightly faster than the assistant could keep up. error: Ref refs/heads/master is at 7074f8e0a11110c532d06746e334f2fec6af6ab4 but expected 95ea86008d72a40d97a81cfc8fb47a0da92166bd fatal: cannot lock HEAD ref Committer crashed: git commit [Param "--allow-empty-message",Param "-m",Param "",Param "--allow-empty",Param "--quiet"] failed Pusher crashed: thread blocked indefinitely in an STM transaction Clearly the the merger ended up running at the same time as the committer, and with both modifying HEAD the committer crashed. I fixed that by making the Merger run its merge inside the annex monad, which avoids it running concurrently with other git operations. Also by making the committer not crash if git fails. What I don't understand is why the pusher then crashed with a STM deadlock. That must be in either the DaemonStatusHandle or the FailedPushMap, and the latter is only used by the pusher. Did the committer's crash somehow break STM? The BlockedIndefinitelyOnSTM exception is described as: -- |The thread is waiting to retry an STM transaction, but there are no -- other references to any @TVar@s involved, so it can't ever continue. If the Committer had a reference to a TVar and crashed, I can sort of see this leading to that exception.. The crash was quite easy to reproduce after the previous commit, but after making the above change, I have yet to see it again. Here's hoping. --- Assistant/Threads/Committer.hs | 7 +++---- Assistant/Threads/Merger.hs | 20 +++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Assistant/Threads/Committer.hs b/Assistant/Threads/Committer.hs index 43bb7b03d9..8092453259 100644 --- a/Assistant/Threads/Committer.hs +++ b/Assistant/Threads/Committer.hs @@ -55,8 +55,7 @@ commitThread st changechan commitchan transferqueue dstatus = thread $ runEvery , "changes" ] void $ alertWhile dstatus commitAlert $ - tryIO (runThreadState st commitStaged) - >> return True + runThreadState st commitStaged recordCommit commitchan (Commit time) else refill readychanges else refill changes @@ -72,10 +71,10 @@ commitThread st changechan commitchan transferqueue dstatus = thread $ runEvery refillChanges changechan cs -commitStaged :: Annex () +commitStaged :: Annex Bool commitStaged = do Annex.Queue.flush - inRepo $ Git.Command.run "commit" + inRepo $ Git.Command.runBool "commit" [ Param "--allow-empty-message" , Param "-m", Param "" -- Empty commits may be made if tree changes cancel diff --git a/Assistant/Threads/Merger.hs b/Assistant/Threads/Merger.hs index c2aa1f52d4..0349bb1f06 100644 --- a/Assistant/Threads/Merger.hs +++ b/Assistant/Threads/Merger.hs @@ -28,7 +28,7 @@ mergeThread st dstatus transferqueue = thread $ do g <- runThreadState st $ fromRepo id let dir = Git.localGitDir g "refs" createDirectoryIfMissing True dir - let hook a = Just $ runHandler st dstatus transferqueue g a + let hook a = Just $ runHandler st dstatus transferqueue a let hooks = mkWatchHooks { addHook = hook onAdd , errHook = hook onErr @@ -38,21 +38,21 @@ mergeThread st dstatus transferqueue = thread $ do where thread = NamedThread thisThread -type Handler = ThreadState -> DaemonStatusHandle -> TransferQueue -> Git.Repo -> FilePath -> Maybe FileStatus -> IO () +type Handler = ThreadState -> DaemonStatusHandle -> TransferQueue -> FilePath -> Maybe FileStatus -> IO () {- Runs an action handler. - - Exceptions are ignored, otherwise a whole thread could be crashed. -} -runHandler :: ThreadState -> DaemonStatusHandle -> TransferQueue -> Git.Repo -> Handler -> FilePath -> Maybe FileStatus -> IO () -runHandler st dstatus transferqueue g handler file filestatus = void $ +runHandler :: ThreadState -> DaemonStatusHandle -> TransferQueue -> Handler -> FilePath -> Maybe FileStatus -> IO () +runHandler st dstatus transferqueue handler file filestatus = void $ either print (const noop) =<< tryIO go where - go = handler st dstatus transferqueue g file filestatus + go = handler st dstatus transferqueue file filestatus {- Called when there's an error with inotify. -} onErr :: Handler -onErr _ _ _ _ msg _ = error msg +onErr _ _ _ msg _ = error msg {- Called when a new branch ref is written. - @@ -66,12 +66,13 @@ onErr _ _ _ _ msg _ = error msg - ran are merged in. -} onAdd :: Handler -onAdd st dstatus transferqueue g file _ +onAdd st dstatus transferqueue file _ | ".lock" `isSuffixOf` file = noop | isAnnexBranch file = runThreadState st $ whenM Annex.Branch.forceUpdate $ queueDeferredDownloads Later transferqueue dstatus - | "/synced/" `isInfixOf` file = mergecurrent =<< Git.Branch.current g + | "/synced/" `isInfixOf` file = runThreadState st $ do + mergecurrent =<< inRepo Git.Branch.current | otherwise = noop where changedbranch = fileToBranch file @@ -83,7 +84,8 @@ onAdd st dstatus transferqueue g file _ , "into" , show current ] - void $ Git.Merge.mergeNonInteractive changedbranch g + void $ inRepo $ + Git.Merge.mergeNonInteractive changedbranch mergecurrent _ = noop equivBranches :: Git.Ref -> Git.Ref -> Bool