assistant: Avoid committer crashing if a file is deleted at the wrong instant.

Assistant/Threads/Committer.hs

@@ -191,7 +191,7 @@ handleAdds delayadd cs = returnWhen (null incomplete) $ do
 				sanitycheck ks $ do
 					key <- liftAnnex $ do
 						showStart "add" $ keyFilename ks
-						Command.Add.ingest ks
+						Command.Add.ingest $ Just ks
 					done (finishedChange change) (keyFilename ks) key
 	  where
 		{- Add errors tend to be transient and will be automatically
@@ -241,7 +241,8 @@ safeToAdd delayadd pending inprocess = do
 	maybe noop (liftIO . threadDelaySeconds) delayadd
 	liftAnnex $ do
 		keysources <- mapM Command.Add.lockDown (map changeFile pending)
-		let inprocess' = map mkinprocess (zip pending keysources)
+		let inprocess' = catMaybes $ 
+			map mkinprocess (zip pending keysources)
 		tmpdir <- fromRepo gitAnnexTmpDir
 		openfiles <- S.fromList . map fst3 . filter openwrite <$>
 			liftIO (Lsof.queryDir tmpdir)
@@ -260,10 +261,11 @@ safeToAdd delayadd pending inprocess = do
 		| S.member (contentLocation ks) openfiles = Left change
 	check _ change = Right change
 
-	mkinprocess (c, ks) = InProcessAddChange
+	mkinprocess (c, Just ks) = Just $ InProcessAddChange
 		{ changeTime = changeTime c
 		, keySource = ks
 		}
+	mkinprocess (_, Nothing) = Nothing
 
 	canceladd (InProcessAddChange { keySource = ks }) = do
 		warning $ keyFilename ks

Command/Add.hs

@@ -58,13 +58,16 @@ start file = ifAnnexed file fixup add
 {- The file that's being added is locked down before a key is generated,
  - to prevent it from being modified in between. It's hard linked into a
  - temporary location, and its writable bits are removed. It could still be
- - written to by a process that already has it open for writing. -}
+ - written to by a process that already has it open for writing.
-lockDown :: FilePath -> Annex KeySource
+ -
+ - Lockdown can fail if a file gets deleted, and Nothing will be returned.
+ -}
+lockDown :: FilePath -> Annex (Maybe KeySource)
 lockDown file = do
-	liftIO $ preventWrite file
 	tmp <- fromRepo gitAnnexTmpDir
 	createAnnexDirectory tmp
-	liftIO $ do
+	liftIO $ catchMaybeIO $ do
+		preventWrite file
 		(tmpfile, h) <- openTempFile tmp (takeFileName file)
 		hClose h
 		nukeFile tmpfile
@@ -76,8 +79,9 @@ lockDown file = do
  - In direct mode, leaves the file alone, and just updates bookkeeping
  - information.
  -}
-ingest :: KeySource -> Annex (Maybe Key)
+ingest :: (Maybe KeySource) -> Annex (Maybe Key)
-ingest source = do
+ingest Nothing = return Nothing
+ingest (Just source) = do
 	backend <- chooseBackend $ keyFilename source
 	ifM isDirect
 		( do

debian/changelog

@@ -19,6 +19,8 @@ git-annex (3.20130108) UNRELEASED; urgency=low
     would also change.
   * webapp: Avoid illegal characters in hostname when creating S3 or
     Glacier remote.
+  * assistant: Avoid committer crashing if a file is deleted at the wrong
+    instant.
 
  -- Joey Hess <joeyh@debian.org>  Tue, 08 Jan 2013 12:37:38 -0400
 

doc/bugs/Committer_crashed.mdwn

@@ -26,3 +26,7 @@ Editing a text file with vim
 # What version of git-annex are you using? On what operating system?
 
 3.20130107 prebuilt tar ball on Debian testing
+
+> Could also fail in `getFileStatus`. In either case it's a race
+> with the file being deleted while it's still in the process of being
+> locked down. Fixed this [[done]] --[[Joey]]