Stale transfer lock and info files will be cleaned up automatically when get/unused/info commands are run.
Deleting lock files is tricky, tricky stuff. I think I got it right!
This commit is contained in:
parent
7299bbb639
commit
7ebf234616
4 changed files with 51 additions and 5 deletions
|
@ -71,7 +71,7 @@ runTransfer' ignorelock t file shouldretry transferobserver transferaction = do
|
||||||
(lck, inprogress) <- liftIO $ prep tfile mode info
|
(lck, inprogress) <- liftIO $ prep tfile mode info
|
||||||
if inprogress && not ignorelock
|
if inprogress && not ignorelock
|
||||||
then do
|
then do
|
||||||
showNote "transfer already in progress"
|
showNote "transfer already in progress, or unable to take transfer lock"
|
||||||
return False
|
return False
|
||||||
else do
|
else do
|
||||||
ok <- retry info metervar $ transferaction meter
|
ok <- retry info metervar $ transferaction meter
|
||||||
|
|
|
@ -123,17 +123,35 @@ startTransferInfo file = TransferInfo
|
||||||
<*> pure file
|
<*> pure file
|
||||||
<*> pure False
|
<*> pure False
|
||||||
|
|
||||||
{- If a transfer is still running, returns its TransferInfo. -}
|
{- If a transfer is still running, returns its TransferInfo.
|
||||||
|
-
|
||||||
|
- If no transfer is running, attempts to clean up the stale
|
||||||
|
- lock and info files. This can happen if a transfer process was
|
||||||
|
- interrupted.
|
||||||
|
-}
|
||||||
checkTransfer :: Transfer -> Annex (Maybe TransferInfo)
|
checkTransfer :: Transfer -> Annex (Maybe TransferInfo)
|
||||||
checkTransfer t = do
|
checkTransfer t = do
|
||||||
tfile <- fromRepo $ transferFile t
|
tfile <- fromRepo $ transferFile t
|
||||||
|
let cleanstale = do
|
||||||
|
void $ tryIO $ removeFile tfile
|
||||||
|
void $ tryIO $ removeFile $ transferLockFile tfile
|
||||||
#ifndef mingw32_HOST_OS
|
#ifndef mingw32_HOST_OS
|
||||||
liftIO $ do
|
liftIO $ do
|
||||||
v <- getLockStatus (transferLockFile tfile)
|
let lck = transferLockFile tfile
|
||||||
|
v <- getLockStatus lck
|
||||||
case v of
|
case v of
|
||||||
Just (pid, _) -> catchDefaultIO Nothing $
|
Just (pid, _) -> catchDefaultIO Nothing $
|
||||||
readTransferInfoFile (Just pid) tfile
|
readTransferInfoFile (Just pid) tfile
|
||||||
Nothing -> return Nothing
|
Nothing -> do
|
||||||
|
-- Take a non-blocking lock while deleting
|
||||||
|
-- the stale lock file.
|
||||||
|
r <- tryLockExclusive Nothing lck
|
||||||
|
case r of
|
||||||
|
Just lockhandle -> do
|
||||||
|
cleanstale
|
||||||
|
dropLock lockhandle
|
||||||
|
_ -> noop
|
||||||
|
return Nothing
|
||||||
#else
|
#else
|
||||||
v <- liftIO $ lockShared $ transferLockFile tfile
|
v <- liftIO $ lockShared $ transferLockFile tfile
|
||||||
liftIO $ case v of
|
liftIO $ case v of
|
||||||
|
@ -141,7 +159,7 @@ checkTransfer t = do
|
||||||
readTransferInfoFile Nothing tfile
|
readTransferInfoFile Nothing tfile
|
||||||
Just lockhandle -> do
|
Just lockhandle -> do
|
||||||
dropLock lockhandle
|
dropLock lockhandle
|
||||||
void $ tryIO $ removeFile $ transferLockFile tfile
|
cleanstale
|
||||||
return Nothing
|
return Nothing
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
|
2
debian/changelog
vendored
2
debian/changelog
vendored
|
@ -16,6 +16,8 @@ git-annex (5.20150508.2) UNRELEASED; urgency=medium
|
||||||
being used.
|
being used.
|
||||||
* Fix an unlikely race that could result in two transfers of the same key
|
* Fix an unlikely race that could result in two transfers of the same key
|
||||||
running at once.
|
running at once.
|
||||||
|
* Stale transfer lock and info files will be cleaned up automatically
|
||||||
|
when get/unused/info commands are run.
|
||||||
|
|
||||||
-- Joey Hess <id@joeyh.name> Mon, 11 May 2015 12:45:06 -0400
|
-- Joey Hess <id@joeyh.name> Mon, 11 May 2015 12:45:06 -0400
|
||||||
|
|
||||||
|
|
|
@ -33,3 +33,29 @@ Oh BTW, this race cannot happen on windows, since on windows, an open lock
|
||||||
file cannot be deleted by another process.
|
file cannot be deleted by another process.
|
||||||
|
|
||||||
> [[fixed|done]]
|
> [[fixed|done]]
|
||||||
|
|
||||||
|
Let's also consider if this change will allow for safely expiring stale
|
||||||
|
lock files..
|
||||||
|
|
||||||
|
Situation before with expiry would have been buggy (which is why it never
|
||||||
|
tried to expire them):
|
||||||
|
|
||||||
|
1. first process creates lock file, takes lock, is interrupted (no more
|
||||||
|
lock)
|
||||||
|
2. second process wants to delete stale lock file; checks if it's locked;
|
||||||
|
isn't
|
||||||
|
3. third process opens existing lock file, prepares to take lock
|
||||||
|
4. second process deletes lock file
|
||||||
|
5. third process takes lock
|
||||||
|
6. third process is now doing a transfer w/o a (visible) lock
|
||||||
|
|
||||||
|
But, after this bug fix, the third process checks if it's lock file
|
||||||
|
exists after taking the lock. Since the second process deleted it,
|
||||||
|
the third process fails with an error "transfer in progress"
|
||||||
|
which is perhaps not accurate, but at least it stopped.
|
||||||
|
|
||||||
|
For this to work though, the second process needs to actually take
|
||||||
|
the lock, in non-blocking mode. If it succeeds, it can keep the lock
|
||||||
|
held while deleting the lock file. This ensures that when the third process
|
||||||
|
takes the lock, the lock file will already be deleted by the time it checks
|
||||||
|
if it's there.
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue