avoid trying to create a content file in order to lock it
The nice refactoring in ec7dd0446a
highlighted a bug in lockContent -- when the content is not present,
this incorrectly created an empty lock file, using the same filename
as the content file.
This seems like it could result in empty objects, which fsck would detect
and complain about. Both drop and move --to call lockContent, as does
Remote.Git.dropKey -- I think we got lucky and this bug didn't show up
because both all of those only operate on files that are present. So
this bug could only manifest if there was a race, and a file's content
was dropped at just the wrong time, just as another process was about to
drop it. (And then only if the other process's dropping failed, otherwise
it'd delete the empty object file.)
Hmm, move --from also called lockContent. Unnecessarily, since the content
is not being removed from the local annex. In this case, the combination of
the 2 bugs could result in an empty lock file being written, and then if
the download of the content failed, left in the object directory as the
content.
This commit also optimises lockContent, avoiding an unncessary
doesFileExist test and instead just catching the exception that's thrown
when the file doesn't exist.
This commit was sponsored by Justine Lam.
This commit is contained in:
parent
ec7dd0446a
commit
e386e26ef2
2 changed files with 10 additions and 12 deletions
|
@ -175,12 +175,10 @@ lockContent key a = do
|
||||||
lock _ (Just lockfile) = createLockFile Nothing lockfile >>= dolock . Just
|
lock _ (Just lockfile) = createLockFile Nothing lockfile >>= dolock . Just
|
||||||
{- Since content files are stored with the write bit disabled, have
|
{- Since content files are stored with the write bit disabled, have
|
||||||
- to fiddle with permissions to open for an exclusive lock. -}
|
- to fiddle with permissions to open for an exclusive lock. -}
|
||||||
opencontentforlock f = catchMaybeIO $ ifM (doesFileExist f)
|
opencontentforlock f = catchDefaultIO Nothing $
|
||||||
( withModifiedFileMode f
|
withModifiedFileMode f
|
||||||
(`unionFileModes` ownerWriteMode)
|
(`unionFileModes` ownerWriteMode)
|
||||||
(createLockFile Nothing f)
|
(openExistingLockFile f)
|
||||||
, createLockFile Nothing f
|
|
||||||
)
|
|
||||||
dolock Nothing = return Nothing
|
dolock Nothing = return Nothing
|
||||||
dolock (Just fd) = do
|
dolock (Just fd) = do
|
||||||
v <- tryIO $ setLock fd (WriteLock, AbsoluteSeek, 0, 0)
|
v <- tryIO $ setLock fd (WriteLock, AbsoluteSeek, 0, 0)
|
||||||
|
|
|
@ -150,11 +150,10 @@ fromOk src key = go =<< Annex.getState Annex.force
|
||||||
return $ u /= Remote.uuid src && elem src remotes
|
return $ u /= Remote.uuid src && elem src remotes
|
||||||
|
|
||||||
fromPerform :: Remote -> Bool -> Key -> AssociatedFile -> CommandPerform
|
fromPerform :: Remote -> Bool -> Key -> AssociatedFile -> CommandPerform
|
||||||
fromPerform src move key afile = moveLock move key $
|
fromPerform src move key afile = ifM (inAnnex key)
|
||||||
ifM (inAnnex key)
|
( dispatch move True
|
||||||
( dispatch move True
|
, dispatch move =<< go
|
||||||
, dispatch move =<< go
|
)
|
||||||
)
|
|
||||||
where
|
where
|
||||||
go = notifyTransfer Download afile $
|
go = notifyTransfer Download afile $
|
||||||
download (Remote.uuid src) key afile noRetry $ \p -> do
|
download (Remote.uuid src) key afile noRetry $ \p -> do
|
||||||
|
@ -166,8 +165,9 @@ fromPerform src move key afile = moveLock move key $
|
||||||
ok <- Remote.removeKey src key
|
ok <- Remote.removeKey src key
|
||||||
next $ Command.Drop.cleanupRemote key src ok
|
next $ Command.Drop.cleanupRemote key src ok
|
||||||
|
|
||||||
{- Locks a key in order for it to be moved.
|
{- Locks a key in order for it to be moved away from the current repository.
|
||||||
- No lock is needed when a key is being copied. -}
|
- No lock is needed when a key is being copied, or moved to the current
|
||||||
|
- repository. -}
|
||||||
moveLock :: Bool -> Key -> Annex a -> Annex a
|
moveLock :: Bool -> Key -> Annex a -> Annex a
|
||||||
moveLock True key a = lockContent key a
|
moveLock True key a = lockContent key a
|
||||||
moveLock False _ a = a
|
moveLock False _ a = a
|
||||||
|
|
Loading…
Reference in a new issue