aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Joey Hess <joey@kitenet.net>2013-01-14 15:02:13 -0400
committerGravatar Joey Hess <joey@kitenet.net>2013-01-14 15:02:13 -0400
commit91c6b94aaf650aba6933a0dec65edeaadc82a14b (patch)
tree96d47507e168ab69e22e382381ee5c3193011f72
parent7efe7b0238b51188acd0ca0340b005c90cbf45ee (diff)
assistant: Avoid committer crashing if a file is deleted at the wrong instant.
-rw-r--r--Assistant/Threads/Committer.hs8
-rw-r--r--Command/Add.hs16
-rw-r--r--debian/changelog2
-rw-r--r--doc/bugs/Committer_crashed.mdwn4
4 files changed, 21 insertions, 9 deletions
diff --git a/Assistant/Threads/Committer.hs b/Assistant/Threads/Committer.hs
index 6e5412e00..46ad83663 100644
--- a/Assistant/Threads/Committer.hs
+++ b/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
diff --git a/Command/Add.hs b/Command/Add.hs
index 883be1b91..1bc38eecd 100644
--- a/Command/Add.hs
+++ b/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. -}
-lockDown :: FilePath -> Annex KeySource
+ - written to by a process that already has it open for writing.
+ -
+ - 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 source = do
+ingest :: (Maybe KeySource) -> Annex (Maybe Key)
+ingest Nothing = return Nothing
+ingest (Just source) = do
backend <- chooseBackend $ keyFilename source
ifM isDirect
( do
diff --git a/debian/changelog b/debian/changelog
index 249164e39..8f9f0150e 100644
--- a/debian/changelog
+++ b/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
diff --git a/doc/bugs/Committer_crashed.mdwn b/doc/bugs/Committer_crashed.mdwn
index 50347b607..caa8b1c50 100644
--- a/doc/bugs/Committer_crashed.mdwn
+++ b/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]]