fsck --from remote -J concurrency bug

fsck --from remote: Fix a concurrency bug that could make it incorrectly
detect that content in the remote is corrupt, and remove it, resulting in
data loss.

diff --git a/CHANGELOG b/CHANGELOG
index 5b624ad376694add6e4f36fc0a3645f398fc5f9f..227f74805e9e77e5a642719b91571b5cbf657b85 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,8 @@
 git-annex (7.20200205) UNRELEASED; urgency=medium
 
+  * fsck --from remote: Fix a concurrency bug that could make it incorrectly
+    detect that content in the remote is corrupt, and remove it, resulting in
+    data loss.
   * When git-annex is built with a ssh that does not support ssh connection
     caching, default annex.sshcaching to false, but let the user override it.
   * Improve warning messages further when ssh connection caching cannot

diff --git a/Command/Fsck.hs b/Command/Fsck.hs
index 65c0112ea76fb32bd59cd4e25d026a279cc20936..cee57c763b141a1026f53b1f40cb76338d36b593 100644 (file)
--- a/Command/Fsck.hs
+++ b/Command/Fsck.hs
@@ -1,6 +1,6 @@
 {- git-annex command
  -
- - Copyright 2010-2019 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2020 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -161,6 +161,11 @@ performRemote key afile backend numcopies remote =
                ]
        ai = mkActionItem (key, afile)
        withtmp a = do
+               -- Put it in the gitAnnexTmpObjectDir since that's on a
+               -- filesystem where object temp files are normally
+               -- stored. The pid prevents multiple fsck processes
+               -- contending over the same file. (Multiple threads cannot,
+               -- because OnlyActionOn is used.)
                pid <- liftIO getPID
                t <- fromRepo gitAnnexTmpObjectDir
                createAnnexDirectory t
@@ -541,7 +546,7 @@ badContentRemote remote localcopy key = do
 
 runFsck :: Incremental -> ActionItem -> Key -> Annex Bool -> CommandStart
 runFsck inc ai key a = stopUnless (needFsck inc key) $
-       starting "fsck" ai $ do
+       starting "fsck" (OnlyActionOn key ai) $ do
                ok <- a
                when ok $
                        recordFsckTime inc key

diff --git a/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail.mdwn b/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail.mdwn
index 80ab6b52eb0b34f89a46a25766e916cf56318be7..d631ae5a4f6523314024006be7f7f1c6f51b4053 100644 (file)
--- a/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail.mdwn
+++ b/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail.mdwn
@@ -145,3 +145,5 @@ whereis file2 (1 copy)
        5d9b0df2-000b-4273-bc4a-fb3b9d8319bd -- testrepo [here]
 ok
 """]]
+
+> [[fixed|done]] --[[Joey]]

diff --git a/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail/comment_1_8cc0d742cd59046b038879f4823c9639._comment b/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail/comment_1_8cc0d742cd59046b038879f4823c9639._comment
new file mode 100644 (file)
index 0000000..755f2bb
--- /dev/null
+++ b/doc/bugs/Parallel_fsck_on_files_with_same_content_in_bup_remote_can_fail/comment_1_8cc0d742cd59046b038879f4823c9639._comment
@@ -0,0 +1,11 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2020-02-14T18:49:07Z"
+ content="""
+Ugh, I think this could potentially result in data loss. Not when using bup, 
+but other special remotes.
+
+I've fixed it in git and will think about moving the date of the next
+release up.
+"""]]