v10 upgrade locking

The v10 upgrade should almost be safe now. What remains to be done is
notice when the v10 upgrade has occurred, while holding the shared lock,
and switch to using v10 lock files.

Sponsored-by: Dartmouth College's Datalad project

Annex/Content.hs

@@ -188,9 +188,8 @@ winLocker _ _ Nothing = (return Nothing, Nothing)
  - the content is not present, because the content file is not always
  - the file that is locked. -}
 lockContentUsing :: ContentLocker -> Key -> Annex a -> Annex a -> Annex a
-lockContentUsing contentlocker key fallback a = do
+lockContentUsing contentlocker key fallback a = withContentLockFile key $ \mlockfile -> do
 	contentfile <- calcRepo (gitAnnexLocation key)
-	mlockfile <- contentLockFile key
 	let (locker, sharedtoexclusive) = contentlocker contentfile mlockfile
 	bracket
 		(lock locker mlockfile)

Annex/Content/Presence.hs

@@ -1,6 +1,6 @@
 {- git-annex object content presence
  -
- - Copyright 2010-2021 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2022 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -17,14 +17,16 @@ module Annex.Content.Presence (
 	isUnmodified,
 	isUnmodified',
 	isUnmodifiedCheap,
-	contentLockFile,
+	withContentLockFile,
 ) where
 
 import Annex.Content.Presence.LowLevel
 import Annex.Common
 import qualified Annex
 import Annex.LockPool
+import Annex.LockFile
 import Annex.Version
+import Types.RepoVersion
 import qualified Database.Keys
 import Annex.InodeSentinal
 import Utility.InodeCache
@@ -77,7 +79,7 @@ inAnnexSafe key = inAnnex' (fromMaybe True) (Just False) go key
 	is_unlocked = Just True
 	is_missing = Just False
 
-	go contentfile = flip checklock contentfile =<< contentLockFile key
+	go contentfile = withContentLockFile key $ flip checklock contentfile
 
 #ifndef mingw32_HOST_OS
 	checklock Nothing contentfile = checkOr is_missing contentfile
@@ -116,20 +118,36 @@ inAnnexSafe key = inAnnex' (fromMaybe True) (Just False) go key
 			)
 #endif
 
-contentLockFile :: Key -> Annex (Maybe RawFilePath)
+{- Runs an action with the lock file to use to lock a key's content.
+ - When the content file itself should be locked, runs the action with
+ - Nothing.
+ -
+ - In v9 and below, while the action is running, a shared lock is held of the
+ - gitAnnexContentLockLock. That prevents the v10 upgrade, which changes how
+ - content locking works, from running at the same time as content is locked
+ - using the old method.
+ -}
+withContentLockFile :: Key -> (Maybe RawFilePath -> Annex a) -> Annex a
+withContentLockFile k a = do
+	v <- getVersion
+	let go = contentLockFile k v >>= a
+	if versionNeedsWritableContentFiles v
+		then withSharedLock gitAnnexContentLockLock go
+		else go
+
+contentLockFile :: Key -> Maybe RepoVersion -> Annex (Maybe RawFilePath)
 #ifndef mingw32_HOST_OS
 {- Older versions of git-annex locked content files themselves, but newer
  - versions use a separate lock file, to better support repos shared
  - amoung users in eg a group. -}
-contentLockFile key = ifM (versionNeedsWritableContentFiles <$> getVersion)
-	( pure Nothing
-	, Just <$> calcRepo (gitAnnexContentLock key)
-	)
+contentLockFile key v
+	| versionNeedsWritableContentFiles v = pure Nothing
+	| otherwise = Just <$> calcRepo (gitAnnexContentLock key)
 #else
 {- Windows always has to use a separate lock file from the content, since
  - locking the actual content file would interfere with the user's
  - use of it. -}
-contentLockFile key = Just <$> calcRepo (gitAnnexContentLock key)
+contentLockFile key _ = Just <$> calcRepo (gitAnnexContentLock key)
 #endif
 
 {- Performs an action, passing it the location to use for a key's content. -}

Annex/Locations.hs

@@ -20,6 +20,7 @@ module Annex.Locations (
 	gitAnnexLink,
 	gitAnnexLinkCanonical,
 	gitAnnexContentLock,
+	gitAnnexContentLockLock,
 	gitAnnexInodeSentinal,
 	gitAnnexInodeSentinalCache,
 	annexLocationsBare,
@@ -239,6 +240,11 @@ gitAnnexContentLock key r config = do
 	loc <- gitAnnexLocation key r config
 	return $ loc <> ".lck"
 
+{- Lock that is held when taking the gitAnnexContentLock to support the v10
+ - upgrade. -}
+gitAnnexContentLockLock :: Git.Repo -> RawFilePath
+gitAnnexContentLockLock r = gitAnnexDir r P.</> "content.lck"
+
 gitAnnexInodeSentinal :: Git.Repo -> RawFilePath
 gitAnnexInodeSentinal r = gitAnnexDir r P.</> "sentinal"
 

Upgrade/V8.hs

@@ -9,7 +9,6 @@ module Upgrade.V8 where
 
 import Annex.Common
 import Types.Upgrade
-import Utility.Daemon
 
 upgrade :: Bool -> Annex UpgradeResult
 upgrade automatic = do

Upgrade/V9.hs

@@ -11,6 +11,8 @@ import Annex.Common
 import Types.Upgrade
 import Annex.Content
 import Annex.Perms
+import Annex.LockFile
+import Annex.Version
 import Git.ConfigTypes
 import Types.RepoVersion
 import Logs.Upgrade
@@ -21,12 +23,12 @@ import Data.Time.Clock.POSIX
 upgrade :: Bool -> Annex UpgradeResult
 upgrade automatic
 	| automatic = do
-		-- For automatic upgrade, wait until a year after the v9
-		-- upgrade. This is to give time for any old processes
-		-- that were running before the v9 upgrade to finish.
-		-- Such old processes lock content using the old method,
-		-- and it is not safe for such to still be running after
-		-- this upgrade.
+		{- For automatic upgrade, wait until a year after the v9
+		 - upgrade. This is to give time for any old processes
+		 - that were running before the v9 upgrade to finish.
+		 - Such old processes lock content using the old method,
+		 - and it is not safe for such to still be running after
+		 - this upgrade. -}
 		timeOfUpgrade (RepoVersion 9) >>= \case
 			Nothing -> performUpgrade automatic
 			Just t -> do
@@ -37,9 +39,9 @@ upgrade automatic
 						performUpgrade automatic
 	| otherwise = performUpgrade automatic
   where
-	-- Skip upgrade when git-annex assistant (or watch) is running,
-	-- because these are long-running daemons that could conceivably
-	-- run for an entire year.
+	{- Skip upgrade when git-annex assistant (or watch) is running,
+	 - because these are long-running daemons that could conceivably
+	 - run for an entire year and so predate the v9 upgrade. -}
 	checkassistantrunning a = do
 		pidfile <- fromRepo gitAnnexPidFile
 		liftIO (checkDaemon (fromRawFilePath pidfile)) >>= \case
@@ -50,19 +52,28 @@ performUpgrade :: Bool -> Annex UpgradeResult
 performUpgrade automatic = do
 	unless automatic $
 		showAction "v9 to v10"
+	
+	{- Take a lock to ensure that there are no other git-annex
+	 - processes running that are using the old content locking method. -}
+	withExclusiveLock gitAnnexContentLockLock $ do
+		{- When core.sharedRepository is set, object files
+		 - used to have their write bits set. That can now be
+		 - removed, if the user the upgrade is running as has
+		 - permission to remove it.
+		 - (Otherwise, a later fsck will fix up the permissions.) -}
+		withShared $ \sr -> case sr of
+			GroupShared -> removewrite sr
+			AllShared -> removewrite sr
+			_ -> return ()
 
-	{- When core.sharedRepository is set, object files
-	 - used to have their write bits set. That can now be removed,
-	 - if the user the upgrade is running as has permission to remove
-	 - it. (Otherwise, a later fsck will fix up the permissions.) -}
-	withShared $ \sr -> case sr of
-		GroupShared -> removewrite sr
-		AllShared -> removewrite sr
-		_ -> return ()
+		{- Set the new version while still holding the lock,
+		 - so that any other process waiting for the lock will
+		 - be able to detect that the upgrade happened. -}
+		setVersion newver
 
-	return UpgradeDeferred
+		return UpgradeSuccess
   where
-	newver = Just (RepoVersion 9)
+	newver = RepoVersion 10
 
 	removewrite sr = do
 		ks <- listKeys InAnnex
@@ -71,7 +82,7 @@ performUpgrade automatic = do
 			keystatus <- getKeyStatus k
 			case keystatus of
 				KeyPresent -> void $ tryIO $
-					freezeContent'' sr obj newver
+					freezeContent'' sr obj (Just newver)
 				KeyUnlockedThin -> return ()
 				KeyLockedThin -> return ()
 				KeyMissing -> return ()

doc/bugs/shared_setting_of_git_causes_annex__39__ed_files_to_be_writeable__33__/comment_12_fc10e403f35f47a6c4464f3f68d01eca._comment

@@ -26,4 +26,11 @@ whenever locking content files. And the v10 upgrade takes an exclusive
 lock. But this seems to fail when a v9 process is running -- if it blocks
 on the shared lock for the v10 upgrade, it would still go on the lock in v9
 mode in the now v10 repository.
+
+Update: That problem can be avoided by re-reading the git config
+to check if v10 was enabled, once it has taken the shared lock.
+That will mean that v9 repos do a little bit more work when locking content
+and dropping. For efficiency, use an InodeCache and only re-read when it's changed.
+This will need the v10 upgrade to set annex.version while it is still
+holding the exclusive lock.
 """]]