Fix bug that prevented transfer locks from working when run on SMB or other filesystem that does not support fcntl locks and hard links.

This commit was sponsored by Ethan Aubin.
This commit is contained in:
Joey Hess 2017-06-06 14:21:43 -04:00
parent fa50906a80
commit ed639c140d
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
4 changed files with 73 additions and 6 deletions

View file

@ -19,6 +19,9 @@ git-annex (6.20170520) UNRELEASED; urgency=medium
* Avoid error about git-annex-shell not being found when
syncing with -J with a git remote where git-annex-shell is not
installed.
* Fix bug that prevented transfer locks from working when
run on SMB or other filesystem that does not support fcntl locks
and hard links.
-- Joey Hess <id@joeyh.name> Wed, 24 May 2017 14:03:40 -0400

View file

@ -122,18 +122,21 @@ tryLock lockfile = trySideLock lockfile $ \sidelock -> do
setFileMode tmp (combineModes readModes)
hPutStr h . show =<< mkPidLock
hClose h
st <- getFileStatus tmp
let failedlock = do
let failedlock st = do
dropLock $ LockHandle tmp st sidelock
return Nothing
let tooklock = return $ Just $ LockHandle lockfile' st sidelock
let tooklock st = return $ Just $ LockHandle lockfile' st sidelock
ifM (linkToLock sidelock tmp lockfile')
( do
nukeFile tmp
tooklock
-- May not have made a hard link, so stat
-- the lockfile
lckst <- getFileStatus lockfile'
tooklock lckst
, do
v <- readPidLock lockfile'
hn <- getHostName
tmpst <- getFileStatus tmp
case v of
Just pl | isJust sidelock && hn == lockingHost pl -> do
-- Since we have the sidelock,
@ -142,8 +145,8 @@ tryLock lockfile = trySideLock lockfile $ \sidelock -> do
-- we know that the pidlock is
-- stale, and can take it over.
rename tmp lockfile'
tooklock
_ -> failedlock
tooklock tmpst
_ -> failedlock tmpst
)
-- Linux's open(2) man page recommends linking a pid lock into place,

View file

@ -245,3 +245,5 @@ and the relevant share:
I'm an extremely regular user of git-annex on OS X and Linux, for several years, using it as a podcatcher and to manage most of my "large file" media. It's one of those "couldn't live without" tools. Thanks for writing it.
(Touched, to ask for comments to be emailed to me)
> [[fixed|done]] --[[Joey]]

View file

@ -0,0 +1,59 @@
[[!comment format=mdwn
username="joey"
subject="""comment 5"""
date="2017-06-06T17:54:42Z"
content="""
Here's the relevant parts of the dtruss:
7003/0x16c3126: 121483 3371 127 open("/Volumes/music/ga-test/.git/annex/locktmp16807282475249\0", 0x20A06, 0x180) = 18 0
...
7003/0x16c3126: 121792 10 6 stat64("/Volumes/music/ga-test/.git/annex/locktmp16807282475249\0", 0x200004940, 0x33) = 0 0
7003/0x16c3126: 122041 2341 235 link("/Volumes/music/ga-test/.git/annex/locktmp16807282475249\0", "/Volumes/music/ga-test/.git/annex/locktmp16807282475249.lnk\0") = -1 Err#45
7003/0x16c3126: 122115 885 49 unlink("/Volumes/music/ga-test/.git/annex/locktmp16807282475249.lnk\0", 0x200004D80, 0x33) = -1 Err#2
7003/0x16c3126: 122126 5 1 sigreturn(0x7FFF56C9E390, 0x1E, 0x33) = 0 Err#-2
7003/0x16c3126: 122247 1050 90 open("/Volumes/music/ga-test/.git/annex/pidlock\0", 0xA01, 0x124) = 18 0
7003/0x16c3126: 122256 4 1 fcntl(0x12, 0x3, 0x0) = 1 0
7003/0x16c3126: 122260 5 2 fstat64(0x12, 0x20001A120, 0x0) = 0 0
7003/0x16c3126: 122273 4 1 ioctl(0x12, 0x4004667A, 0x7FFF56C9E3FC) = -1 Err#25
7003/0x16c3126: 122274 2 0 ioctl(0x12, 0x40487413, 0x7FFF56C9E400) = -1 Err#25
7003/0x16c3126: 122314 677 32 open("/Volumes/music/ga-test/.git/annex/locktmp16807282475249\0", 0x20004, 0x1B6) = 19 0
7003/0x16c3126: 122319 5 2 fstat64(0x13, 0x20001A310, 0x1B6) = 0 0
7003/0x16c3126: 122326 2 0 ioctl(0x13, 0x4004667A, 0x7FFF56C9E3FC) = -1 Err#25
7003/0x16c3126: 122327 2 0 ioctl(0x13, 0x40487413, 0x7FFF56C9E400) = -1 Err#25
7003/0x16c3126: 122381 1455 46 read(0x13, "PidLock {lockingPid = 7003, lockingHost = \"ashram\"}\0", 0x1FA0) = 51 0
7003/0x16c3126: 122392 5 2 read(0x13, "\0", 0x1FA0) = 0 0
7003/0x16c3126: 122427 1339 27 close(0x13) = 0 0
7003/0x16c3126: 122441 10 4 select(0x13, 0x7FFF56C9E340, 0x7FFF56C9E3C0, 0x0, 0x7FFF56C9E330) = 1 0
7003/0x16c3126: 122469 661 23 write(0x12, "PidLock {lockingPid = 7003, lockingHost = \"ashram\"}\0", 0x33) = 51 0
7003/0x16c3126: 122577 6028 99 close(0x12) = 0 0
Avove shows hard linking failed, and it fell back to using open to create
the pidlock file. That seemed to finish successfully.
7003/0x16c3126: 122583 4 0 sigreturn(0x7FFF56C9E3B0, 0x1E, 0x33) = 0 Err#-2
Unsure what this is?
7003/0x16c3126: 122697 2126 101 unlink("/Volumes/music/ga-test/.git/annex/locktmp16807282475249\0", 0x1E, 0x33) = 0 0
Here linkToLock must has succeeded, because it deletes the tmp file that was passed to that function.
So, it looks like tryLock succeeded.
7003/0x16c3126: 122746 1423 28 stat64(".git/annex/pidlock\0", 0x20001A510, 0x33) = 0 0
7003/0x16c3126: 122775 10 5 select(0x2, 0x7FFF56C9E340, 0x7FFF56C9E3C0, 0x0, 0x7FFF56C9E330) = 1 0
7003/0x16c3126: 122786 10 8 write(0x1, "(transfer already in progress, or unable to take transfer lock) \0", 0x40) = 64 0
I think this is a call to checkSaneLock, which calls getFileStatus,
and since the pid lock was created earlier, that succeeds.
runTransfer does call checkSaneLock, and if it fails
we get the "transfer already in progress, or unable to take transfer lock"
message.
So, why did checkSaneLock fail?
Well, the device and inode of the lock file
are compared with the device and inode of the temp file
that was passed to linkToLock. However, since the hard link didn't
happen, the device and inode will be different!
And that's the bug.
"""]]