diff --git a/Annex/Content.hs b/Annex/Content.hs index 141f8e206a..c81dc5bff6 100644 --- a/Annex/Content.hs +++ b/Annex/Content.hs @@ -169,13 +169,13 @@ type ContentLocker = RawFilePath -> Maybe LockFile -> (Annex (Maybe LockHandle), posixLocker :: (Maybe FileMode -> LockFile -> Annex (Maybe LockHandle)) -> LockFile -> Annex (Maybe LockHandle) posixLocker takelock lockfile = do mode <- annexFileMode - modifyContent lockfile $ + modifyContentDirWhenExists lockfile $ takelock (Just mode) lockfile #else winLocker :: (LockFile -> IO (Maybe LockHandle)) -> ContentLocker winLocker takelock _ (Just lockfile) = let lck = do - modifyContent lockfile $ + modifyContentDirWhenExists lockfile $ void $ liftIO $ tryIO $ writeFile (fromRawFilePath lockfile) "" liftIO $ takelock lockfile @@ -407,7 +407,7 @@ moveAnnex key af src = ifM (checkSecureHashes' key) where storeobject dest = ifM (liftIO $ R.doesPathExist dest) ( alreadyhave - , adjustedBranchRefresh af $ modifyContent dest $ do + , adjustedBranchRefresh af $ modifyContentDir dest $ do liftIO $ moveFile (fromRawFilePath src) (fromRawFilePath dest) @@ -452,7 +452,7 @@ linkToAnnex :: Key -> RawFilePath -> Maybe InodeCache -> Annex LinkAnnexResult linkToAnnex key src srcic = ifM (checkSecureHashes' key) ( do dest <- calcRepo (gitAnnexLocation key) - modifyContent dest $ linkAnnex To key src srcic dest Nothing + modifyContentDir dest $ linkAnnex To key src srcic dest Nothing , return LinkAnnexFailed ) @@ -528,7 +528,7 @@ linkAnnex fromto key src (Just srcic) dest destmode = unlinkAnnex :: Key -> Annex () unlinkAnnex key = do obj <- calcRepo (gitAnnexLocation key) - modifyContent obj $ do + modifyContentDir obj $ do secureErase obj liftIO $ removeWhenExistsWith R.removeLink obj diff --git a/Annex/Content/Presence.hs b/Annex/Content/Presence.hs index 48c8b92873..86fde17e44 100644 --- a/Annex/Content/Presence.hs +++ b/Annex/Content/Presence.hs @@ -112,7 +112,7 @@ inAnnexSafe key = inAnnex' (fromMaybe True) (Just False) go key - remove the lock file to clean up after ourselves. -} checklock (Just lockfile) contentfile = ifM (liftIO $ doesFileExist (fromRawFilePath contentfile)) - ( modifyContent lockfile $ liftIO $ + ( modifyContentDirWhenExists lockfile $ liftIO $ lockShared lockfile >>= \case Nothing -> return is_locked Just lockhandle -> do diff --git a/Annex/Perms.hs b/Annex/Perms.hs index 4cecc48e79..5e3de2d16a 100644 --- a/Annex/Perms.hs +++ b/Annex/Perms.hs @@ -26,7 +26,8 @@ module Annex.Perms ( createContentDir, freezeContentDir, thawContentDir, - modifyContent, + modifyContentDir, + modifyContentDirWhenExists, withShared, hasFreezeHook, hasThawHook, @@ -290,15 +291,24 @@ createContentDir dest = do dir = parentDir dest {- Creates the content directory for a file if it doesn't already exist, - - or thaws it if it does, then runs an action to modify the file, and - - finally, freezes the content directory. -} -modifyContent :: RawFilePath -> Annex a -> Annex a -modifyContent f a = do + - or thaws it if it does, then runs an action to modify a file in the + - directory, and finally, freezes the content directory. -} +modifyContentDir :: RawFilePath -> Annex a -> Annex a +modifyContentDir f a = do createContentDir f -- also thaws it v <- tryNonAsync a freezeContentDir f either throwM return v +{- Like modifyContentDir, but avoids creating the content directory if it + - ddoes not already exist. In that case, the action will probably fail. -} +modifyContentDirWhenExists :: RawFilePath -> Annex a -> Annex a +modifyContentDirWhenExists f a = do + thawContentDir f + v <- tryNonAsync a + freezeContentDir f + either throwM return v + hasFreezeHook :: Annex Bool hasFreezeHook = isJust . annexFreezeContentCommand <$> Annex.getGitConfig diff --git a/Command/Fix.hs b/Command/Fix.hs index 109b17389e..581cb7ced2 100644 --- a/Command/Fix.hs +++ b/Command/Fix.hs @@ -78,7 +78,7 @@ breakHardLink file key obj = do error "unable to break hard link" thawContent tmp' Database.Keys.storeInodeCaches key [tmp'] - modifyContent obj $ freezeContent obj + modifyContentDir obj $ freezeContent obj next $ return True makeHardLink :: RawFilePath -> Key -> CommandPerform diff --git a/Command/Lock.hs b/Command/Lock.hs index 620da40133..e0d4b8f885 100644 --- a/Command/Lock.hs +++ b/Command/Lock.hs @@ -77,13 +77,13 @@ perform file key = do breakhardlink obj = whenM (catchBoolIO $ (> 1) . linkCount <$> liftIO (R.getFileStatus obj)) $ do mfc <- withTSDelta (liftIO . genInodeCache file) unlessM (sameInodeCache obj (maybeToList mfc)) $ do - modifyContent obj $ replaceGitAnnexDirFile (fromRawFilePath obj) $ \tmp -> do + modifyContentDir obj $ replaceGitAnnexDirFile (fromRawFilePath obj) $ \tmp -> do unlessM (checkedCopyFile key obj (toRawFilePath tmp) Nothing) $ giveup "unable to lock file" Database.Keys.storeInodeCaches key [obj] -- Try to repopulate obj from an unmodified associated file. - repopulate obj = modifyContent obj $ do + repopulate obj = modifyContentDir obj $ do g <- Annex.gitRepo fs <- map (`fromTopFilePath` g) <$> Database.Keys.getAssociatedFiles key diff --git a/Upgrade/V5/Direct.hs b/Upgrade/V5/Direct.hs index 9903f84d07..49665e8773 100644 --- a/Upgrade/V5/Direct.hs +++ b/Upgrade/V5/Direct.hs @@ -97,7 +97,7 @@ associatedFilesRelative key = do removeAssociatedFiles :: Key -> Annex () removeAssociatedFiles key = do mapping <- calcRepo $ gitAnnexMapping key - modifyContent mapping $ + modifyContentDir mapping $ liftIO $ removeWhenExistsWith R.removeLink mapping {- Checks if a file in the tree, associated with a key, has not been modified. @@ -124,7 +124,7 @@ recordedInodeCache key = withInodeCacheFile key $ \f -> {- Removes an inode cache. -} removeInodeCache :: Key -> Annex () removeInodeCache key = withInodeCacheFile key $ \f -> - modifyContent f $ + modifyContentDir f $ liftIO $ removeWhenExistsWith R.removeLink f withInodeCacheFile :: Key -> (RawFilePath -> Annex a) -> Annex a diff --git a/doc/todo/command_to___34__migrate__34___from_adjusted_mode/comment_6_e8ab74bb4951fe7f0c71630bb278ad6f._comment b/doc/todo/command_to___34__migrate__34___from_adjusted_mode/comment_6_e8ab74bb4951fe7f0c71630bb278ad6f._comment new file mode 100644 index 0000000000..3b17573088 --- /dev/null +++ b/doc/todo/command_to___34__migrate__34___from_adjusted_mode/comment_6_e8ab74bb4951fe7f0c71630bb278ad6f._comment @@ -0,0 +1,26 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 6""" + date="2022-05-16T15:24:43Z" + content=""" +If fsck locks the content for removal, then moves it to the preferred +location, how is that any different from git-annex first dropping content +and then very quickly retrieving another copy and storing it in the other +location? The only difference is timing, but things like being suspended +and resumed can affect timing. + +So, if there is a problem with fsck doing that, there would also be a more +general problem, that could occur in other circumstances, even if only +rarely. + +One way to see the general problem happen would be to have two processes +trying to drop the same object. One process finds the object location, then +stalls. Meanwhile, the second process drops the object. Then the first +process resumes, and locks for removal. Per comment #5 this will result in +a dangling lock file in the object directory. I have not managed to get +this to happen yet though. + +A fix for the general problem is to make it not create the +object directory when opening the object lock file. So I've made that +change. +"""]]