diff --git a/weed/mount/filehandle.go b/weed/mount/filehandle.go index 072d4a64ff..cb530614d0 100644 --- a/weed/mount/filehandle.go +++ b/weed/mount/filehandle.go @@ -22,6 +22,7 @@ var IsDebugFileReadWrite = false type FileHandle struct { fh FileHandleId counter int64 + fhs map[FileHandleId]struct{} // live fh values, one per open; guarded by fhMap's lock entry *LockedEntry entryLock sync.RWMutex entryChunkGroup *filer.ChunkGroup @@ -79,6 +80,7 @@ func newFileHandle(wfs *WFS, handleId FileHandleId, inode uint64, entry *filer_p fh := &FileHandle{ fh: handleId, counter: 1, + fhs: map[FileHandleId]struct{}{handleId: {}}, inode: inode, wfs: wfs, } diff --git a/weed/mount/filehandle_map.go b/weed/mount/filehandle_map.go index 9bc585bf62..0fcf5196e5 100644 --- a/weed/mount/filehandle_map.go +++ b/weed/mount/filehandle_map.go @@ -54,7 +54,10 @@ func (i *FileHandleToInode) MarkInodeRenamed(inode uint64) { // existed. An existing handle only gets its counter bumped: its entry belongs // to whoever holds the handle lock, so callers install there (AcquireHandle) // rather than under this one. -func (i *FileHandleToInode) AcquireFileHandle(wfs *WFS, inode uint64, entry *filer_pb.Entry, versionTsNs int64, signature int32) (*FileHandle, bool) { +// Every open gets its own fh value over the shared FileHandle so that +// RELEASE — which identifies nothing else about the dying description — can +// scope per-open state (e.g. OFD lock owners) to the open that is closing. +func (i *FileHandleToInode) AcquireFileHandle(wfs *WFS, inode uint64, entry *filer_pb.Entry, versionTsNs int64, signature int32) (*FileHandle, FileHandleId, bool) { i.Lock() defer i.Unlock() fh, found := i.inode2fh[inode] @@ -64,10 +67,13 @@ func (i *FileHandleToInode) AcquireFileHandle(wfs *WFS, inode uint64, entry *fil fh.entryVersionSignature.Store(signature) i.inode2fh[inode] = fh i.fh2inode[fh.fh] = inode - return fh, false + return fh, fh.fh, false } fh.counter++ - return fh, true + openFh := FileHandleId(util.RandomUint64()) + fh.fhs[openFh] = struct{}{} + i.fh2inode[openFh] = inode + return fh, openFh, true } func (i *FileHandleToInode) ReleaseByHandle(fh FileHandleId) *FileHandle { @@ -78,12 +84,15 @@ func (i *FileHandleToInode) ReleaseByHandle(fh FileHandleId) *FileHandle { if !found { return nil } + // The released id never resolves again: a duplicate RELEASE on it must + // not decrement the counter a second time. + delete(i.fh2inode, fh) fhHandle, fhFound := i.inode2fh[inode] if !fhFound { - delete(i.fh2inode, fh) return nil } + delete(fhHandle.fhs, fh) // If the counter is already <= 0, a prior Release already started the // drain. Return nil to prevent double-processing. @@ -99,7 +108,10 @@ func (i *FileHandleToInode) ReleaseByHandle(fh FileHandleId) *FileHandle { return fhHandle } delete(i.inode2fh, inode) - delete(i.fh2inode, fhHandle.fh) + for openFh := range fhHandle.fhs { + delete(i.fh2inode, openFh) + } + fhHandle.fhs = nil return fhHandle } return nil @@ -111,6 +123,27 @@ func (i *FileHandleToInode) ReleaseByHandle(fh FileHandleId) *FileHandle { func (i *FileHandleToInode) RemoveFileHandle(fh FileHandleId, inode uint64) { i.Lock() defer i.Unlock() + if fhHandle, ok := i.inode2fh[inode]; ok { + for openFh := range fhHandle.fhs { + delete(i.fh2inode, openFh) + } + } delete(i.inode2fh, inode) delete(i.fh2inode, fh) } + +// openFhIds returns the live fh values minted for an inode's shared handle — +// one per still-open description. +func (i *FileHandleToInode) openFhIds(inode uint64) []FileHandleId { + i.RLock() + defer i.RUnlock() + fh := i.inode2fh[inode] + if fh == nil { + return nil + } + ids := make([]FileHandleId, 0, len(fh.fhs)) + for id := range fh.fhs { + ids = append(ids, id) + } + return ids +} diff --git a/weed/mount/filehandle_read_remote_test.go b/weed/mount/filehandle_read_remote_test.go index 70d353b2ef..20fc5d06a8 100644 --- a/weed/mount/filehandle_read_remote_test.go +++ b/weed/mount/filehandle_read_remote_test.go @@ -118,7 +118,7 @@ func TestReadUncachedRemoteEntryDoesNotDeadlock(t *testing.T) { t.Cleanup(func() { wfs.metaCache.Shutdown() }) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: uint64(len(testServer.content))}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: int64(len(testServer.content))}, diff --git a/weed/mount/posix_file_lock.go b/weed/mount/posix_file_lock.go index 425083f5e4..9cf77ef789 100644 --- a/weed/mount/posix_file_lock.go +++ b/weed/mount/posix_file_lock.go @@ -41,6 +41,53 @@ type lockWaiter struct { wakeRefHeld bool } +// fhLockOwners maps each open's fh value to the non-flock lock owners that +// acquired locks through it. The kernel scopes POSIX fcntl locks to the +// process — released on FLUSH by LockOwner — while OFD fcntl locks are scoped +// to the open file description and get no cleanup notice: RELEASE carries a +// lock_owner only for flock holders. Tracking owners per open lets Release +// drop the locks of the description that is closing while sibling opens of +// the same inode keep theirs. +type fhLockOwners struct { + mu sync.Mutex + m map[uint64]map[uint64]struct{} +} + +func (l *fhLockOwners) add(fh, owner uint64) { + l.mu.Lock() + defer l.mu.Unlock() + if l.m == nil { + l.m = make(map[uint64]map[uint64]struct{}) + } + owners := l.m[fh] + if owners == nil { + owners = make(map[uint64]struct{}) + l.m[fh] = owners + } + owners[owner] = struct{}{} +} + +func (l *fhLockOwners) pop(fh uint64) map[uint64]struct{} { + l.mu.Lock() + defer l.mu.Unlock() + owners := l.m[fh] + delete(l.m, fh) + return owners +} + +func (l *fhLockOwners) remove(fh, owner uint64) { + l.mu.Lock() + defer l.mu.Unlock() + owners := l.m[fh] + if owners == nil { + return + } + delete(owners, owner) + if len(owners) == 0 { + delete(l.m, fh) + } +} + // PosixLockTable is the per-mount POSIX lock manager. type PosixLockTable struct { mu sync.Mutex diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index 7ee15a2eed..1385017eed 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -160,6 +160,7 @@ type WFS struct { posixSid uint64 // this mount's session id, for routed-lock owner identity posixHint *posixLockHint // local fcntl-lock hint for routed mode posixOwn *posixlock.Manager // mirror of locks this mount holds, re-asserted via keepalive + fhLocks fhLockOwners // non-flock lock owners per open fh, drained at release peerRegistrar *PeerRegistrar peerDirectory *PeerDirectory peerGrpcServer *PeerGrpcServer diff --git a/weed/mount/weedfs_file_copy_range_test.go b/weed/mount/weedfs_file_copy_range_test.go index 4bca12e944..a91cfca78f 100644 --- a/weed/mount/weedfs_file_copy_range_test.go +++ b/weed/mount/weedfs_file_copy_range_test.go @@ -21,7 +21,7 @@ func TestWholeFileServerCopyCandidate(t *testing.T) { srcInode := wfs.inodeToPath.Lookup(srcPath, 1, false, false, 0, true) dstInode := wfs.inodeToPath.Lookup(dstPath, 1, false, false, 0, true) - srcHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ + srcHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ Name: "src.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -30,7 +30,7 @@ func TestWholeFileServerCopyCandidate(t *testing.T) { }, Content: []byte("hello"), }, 0, 0) - dstHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ + dstHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ Name: "dst.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -86,7 +86,7 @@ func TestCopyFileRangeUsesServerSideWholeFileCopy(t *testing.T) { srcInode := wfs.inodeToPath.Lookup(srcPath, 1, false, false, 0, true) dstInode := wfs.inodeToPath.Lookup(dstPath, 1, false, false, 0, true) - srcHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ + srcHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ Name: "src.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -95,7 +95,7 @@ func TestCopyFileRangeUsesServerSideWholeFileCopy(t *testing.T) { }, Content: []byte("hello"), }, 0, 0) - dstHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ + dstHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ Name: "dst.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -171,7 +171,7 @@ func TestCopyFileRangeDoesNotFallbackAfterCommittedServerCopyRefreshFailure(t *t srcInode := wfs.inodeToPath.Lookup(srcPath, 1, false, false, 0, true) dstInode := wfs.inodeToPath.Lookup(dstPath, 1, false, false, 0, true) - srcHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ + srcHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ Name: "src.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -182,7 +182,7 @@ func TestCopyFileRangeDoesNotFallbackAfterCommittedServerCopyRefreshFailure(t *t }, Content: []byte("hello"), }, 0, 0) - dstHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ + dstHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ Name: "dst.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100600, @@ -253,7 +253,7 @@ func TestCopyFileRangeReturnsEIOForAmbiguousServerSideCopy(t *testing.T) { srcInode := wfs.inodeToPath.Lookup(srcPath, 1, false, false, 0, true) dstInode := wfs.inodeToPath.Lookup(dstPath, 1, false, false, 0, true) - srcHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ + srcHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ Name: "src.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, @@ -262,7 +262,7 @@ func TestCopyFileRangeReturnsEIOForAmbiguousServerSideCopy(t *testing.T) { }, Content: []byte("hello"), }, 0, 0) - dstHandle, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ + dstHandle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ Name: "dst.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100600, diff --git a/weed/mount/weedfs_file_fallocate_test.go b/weed/mount/weedfs_file_fallocate_test.go index a0e8c934f1..d565e21bf6 100644 --- a/weed/mount/weedfs_file_fallocate_test.go +++ b/weed/mount/weedfs_file_fallocate_test.go @@ -22,7 +22,7 @@ func newOpenFileHandle(t *testing.T, fileSize uint64) (*WFS, *FileHandle) { wfs := newCopyRangeTestWFS() path := util.FullPath("/file.txt") inode := wfs.inodeToPath.Lookup(path, 1, false, false, 0, true) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644, diff --git a/weed/mount/weedfs_file_io.go b/weed/mount/weedfs_file_io.go index e38d894b2e..fb1774f565 100644 --- a/weed/mount/weedfs_file_io.go +++ b/weed/mount/weedfs_file_io.go @@ -65,11 +65,12 @@ import ( */ func (wfs *WFS) Open(cancel <-chan struct{}, in *fuse.OpenIn, out *fuse.OpenOut) (status fuse.Status) { var fileHandle *FileHandle + var openFh FileHandleId ctx, cancelFunc := ctxFromCancel(cancel) defer cancelFunc() - fileHandle, status = wfs.AcquireHandle(ctx, in.NodeId, in.Flags, in.Uid, in.Gid) + fileHandle, openFh, status = wfs.AcquireHandle(ctx, in.NodeId, in.Flags, in.Uid, in.Gid) if status == fuse.OK { - out.Fh = uint64(fileHandle.fh) + out.Fh = uint64(openFh) out.OpenFlags = 0 // For read-only opens, set FOPEN_KEEP_CACHE when the file's mtime @@ -140,12 +141,17 @@ func (wfs *WFS) invalidateOpenMtimeCache(inode uint64) { } func (wfs *WFS) Release(cancel <-chan struct{}, in *fuse.ReleaseIn) { - // Drop the file description's flock locks first: close() frees them - // regardless of how writeback fares, and a flush stuck on an unreachable - // filer must not pin a dead holder's lock in the table. + // Drop the file description's locks first: close() frees them regardless + // of how writeback fares, and a flush stuck on an unreachable filer must + // not pin a dead holder's lock in the table. The kernel only reports a + // lock_owner for flock holders, so OFD and leftover POSIX locks are + // drained through the owners recorded for this open's fh value. if in.ReleaseFlags&fuse.FUSE_RELEASE_FLOCK_UNLOCK != 0 { wfs.releaseFlockOwner(in.NodeId, in.LockOwner) } + for owner := range wfs.fhLocks.pop(in.Fh) { + wfs.releasePosixOwner(in.NodeId, owner) + } // Flush is usually sent before Release, but the FUSE protocol does not // guarantee it. Route every Release through doFlush so a dirty handle // (e.g. a deferred create with no intervening Flush) is not dropped. diff --git a/weed/mount/weedfs_file_lock.go b/weed/mount/weedfs_file_lock.go index b593bf72a8..b56366ccac 100644 --- a/weed/mount/weedfs_file_lock.go +++ b/weed/mount/weedfs_file_lock.go @@ -26,36 +26,66 @@ func (wfs *WFS) GetLk(cancel <-chan struct{}, in *fuse.LkIn, out *fuse.LkOut) fu // SetLk sets or clears a POSIX lock (non-blocking). // Returns EAGAIN if the lock conflicts with an existing lock from another owner. func (wfs *WFS) SetLk(cancel <-chan struct{}, in *fuse.LkIn) fuse.Status { + var status fuse.Status if wfs.crossMountLocks() { - return wfs.routedSetLk(cancel, in) + status = wfs.routedSetLk(cancel, in) + } else { + lk := lockRange{ + Start: in.Lk.Start, + End: in.Lk.End, + Typ: in.Lk.Typ, + Owner: in.Owner, + Pid: in.Lk.Pid, + IsFlock: in.LkFlags&fuse.FUSE_LK_FLOCK != 0, + } + status = wfs.posixLocks.SetLk(in.NodeId, lk) } - lk := lockRange{ - Start: in.Lk.Start, - End: in.Lk.End, - Typ: in.Lk.Typ, - Owner: in.Owner, - Pid: in.Lk.Pid, - IsFlock: in.LkFlags&fuse.FUSE_LK_FLOCK != 0, - } - return wfs.posixLocks.SetLk(in.NodeId, lk) + wfs.noteFhLockOwner(in, status) + return status } // SetLkw sets a POSIX lock (blocking). // Waits until the lock can be acquired or the request is cancelled. func (wfs *WFS) SetLkw(cancel <-chan struct{}, in *fuse.LkIn) fuse.Status { + var status fuse.Status if wfs.crossMountLocks() { - return wfs.routedSetLkw(cancel, in) + status = wfs.routedSetLkw(cancel, in) + } else { + lk := lockRange{ + Start: in.Lk.Start, + End: in.Lk.End, + Typ: in.Lk.Typ, + Owner: in.Owner, + Pid: in.Lk.Pid, + IsFlock: in.LkFlags&fuse.FUSE_LK_FLOCK != 0, + } + if lk.Typ == f_UNLCK { + status = wfs.posixLocks.SetLk(in.NodeId, lk) + } else { + status = wfs.posixLocks.SetLkw(in.NodeId, lk, cancel) + } + } + wfs.noteFhLockOwner(in, status) + return status +} + +// noteFhLockOwner remembers a granted non-flock lock owner under the open it +// was taken on, so Release can drain every lock the file description held — +// the kernel sends no cleanup notice for OFD locks. +func (wfs *WFS) noteFhLockOwner(in *fuse.LkIn, status fuse.Status) { + if status != fuse.OK || in.Lk.Typ == f_UNLCK || in.LkFlags&fuse.FUSE_LK_FLOCK != 0 { + return + } + wfs.fhLocks.add(in.Fh, in.Owner) +} + +// forgetLockOwner drops a lock owner from every recorded open of the inode. +// Flush calls it only after the owner's process-wide locks were released, so +// keeping the record would let a later Release of an unrelated open drop +// locks the process acquired in between — and a failed release keeps its +// record so Release can retry. +func (wfs *WFS) forgetLockOwner(inode, owner uint64) { + for _, openFh := range wfs.fhMap.openFhIds(inode) { + wfs.fhLocks.remove(uint64(openFh), owner) } - lk := lockRange{ - Start: in.Lk.Start, - End: in.Lk.End, - Typ: in.Lk.Typ, - Owner: in.Owner, - Pid: in.Lk.Pid, - IsFlock: in.LkFlags&fuse.FUSE_LK_FLOCK != 0, - } - if lk.Typ == f_UNLCK { - return wfs.posixLocks.SetLk(in.NodeId, lk) - } - return wfs.posixLocks.SetLkw(in.NodeId, lk, cancel) } diff --git a/weed/mount/weedfs_file_mkrm.go b/weed/mount/weedfs_file_mkrm.go index 0d6177aaa7..f982f9a2fe 100644 --- a/weed/mount/weedfs_file_mkrm.go +++ b/weed/mount/weedfs_file_mkrm.go @@ -54,20 +54,20 @@ func (wfs *WFS) Create(cancel <-chan struct{}, in *fuse.CreateIn, name string, o return fuse.Status(syscall.EEXIST) } inode = wfs.inodeToPath.Lookup(entryFullPath, newEntry.Attributes.Crtime, false, len(newEntry.HardLinkId) > 0, newEntry.Attributes.Inode, true) - fileHandle, status := wfs.AcquireHandle(ctx, inode, in.Flags, in.Uid, in.Gid) + fileHandle, openFh, status := wfs.AcquireHandle(ctx, inode, in.Flags, in.Uid, in.Gid) if status != fuse.OK { return status } if in.Flags&syscall.O_TRUNC != 0 && in.Flags&fuse.O_ANYWRITE != 0 { if code = wfs.truncateEntry(entryFullPath, newEntry); code != fuse.OK { - wfs.ReleaseHandle(fileHandle.fh) + wfs.ReleaseHandle(openFh) return code } newEntry = fileHandle.GetEntry().GetEntry() } wfs.outputPbEntry(&out.EntryOut, inode, newEntry) - out.Fh = uint64(fileHandle.fh) + out.Fh = uint64(openFh) out.OpenFlags = 0 return fuse.OK } @@ -103,19 +103,19 @@ func (wfs *WFS) Create(cancel <-chan struct{}, in *fuse.CreateIn, name string, o return fuse.EIO } inode = wfs.inodeToPath.Lookup(entryFullPath, newEntry.Attributes.Crtime, false, len(newEntry.HardLinkId) > 0, newEntry.Attributes.Inode, true) - fileHandle, status := wfs.AcquireHandle(ctx, inode, in.Flags, in.Uid, in.Gid) + fileHandle, openFh, status := wfs.AcquireHandle(ctx, inode, in.Flags, in.Uid, in.Gid) if status != fuse.OK { return status } if in.Flags&syscall.O_TRUNC != 0 && in.Flags&fuse.O_ANYWRITE != 0 { if code = wfs.truncateEntry(entryFullPath, newEntry); code != fuse.OK { - wfs.ReleaseHandle(fileHandle.fh) + wfs.ReleaseHandle(openFh) return code } newEntry = fileHandle.GetEntry().GetEntry() } wfs.outputPbEntry(&out.EntryOut, inode, newEntry) - out.Fh = uint64(fileHandle.fh) + out.Fh = uint64(openFh) out.OpenFlags = 0 return fuse.OK } else if code != fuse.OK { @@ -132,7 +132,7 @@ func (wfs *WFS) Create(cancel <-chan struct{}, in *fuse.CreateIn, name string, o // For deferred creates, bypass AcquireHandle (which calls maybeReadEntry // and would fail since the entry is not yet on the filer or in the meta cache). // We already have the entry from createRegularFile, so create the handle directly. - fileHandle, existed := wfs.fhMap.AcquireFileHandle(wfs, inode, newEntry, 0, 0) + fileHandle, openFh, existed := wfs.fhMap.AcquireFileHandle(wfs, inode, newEntry, 0, 0) if existed { // A create is authoritative for the entry it just made, so it takes // effect even on a handle that outlived a previous incarnation. @@ -155,7 +155,7 @@ func (wfs *WFS) Create(cancel <-chan struct{}, in *fuse.CreateIn, name string, o } } - out.Fh = uint64(fileHandle.fh) + out.Fh = uint64(openFh) out.OpenFlags = 0 return fuse.OK diff --git a/weed/mount/weedfs_file_mkrm_test.go b/weed/mount/weedfs_file_mkrm_test.go index 6ed637f608..a8d8a2568e 100644 --- a/weed/mount/weedfs_file_mkrm_test.go +++ b/weed/mount/weedfs_file_mkrm_test.go @@ -267,7 +267,7 @@ func TestTruncateEntryClearsDirtyPagesForOpenHandle(t *testing.T) { }, } - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, entry, 0, 0) + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, entry, 0, 0) fh.RememberPath(fullPath) if err := fh.dirtyPages.AddPage(0, []byte("hello"), true, time.Now().UnixNano()); err != nil { @@ -315,7 +315,7 @@ func TestAccessChecksPermissions(t *testing.T) { fullPath := util.FullPath("/visible.txt") inode := wfs.inodeToPath.Lookup(fullPath, 1, false, false, 0, true) - handle, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + handle, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "visible.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0o640, @@ -528,7 +528,7 @@ func TestAcquireHandleHonorsDefaultPermissions(t *testing.T) { } inode := wfs.inodeToPath.Lookup(util.FullPath("/secret.txt"), entry.Attributes.Crtime, false, false, entry.Attributes.Inode, true) - fh, status := wfs.AcquireHandle(context.Background(), inode, syscall.O_RDONLY, 999, 999) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, syscall.O_RDONLY, 999, 999) if status != tc.want { t.Fatalf("AcquireHandle status = %v, want %v", status, tc.want) } diff --git a/weed/mount/weedfs_file_sync.go b/weed/mount/weedfs_file_sync.go index 8c7d852422..8e58e2f23c 100644 --- a/weed/mount/weedfs_file_sync.go +++ b/weed/mount/weedfs_file_sync.go @@ -65,8 +65,8 @@ func (wfs *WFS) Flush(cancel <-chan struct{}, in *fuse.FlushIn) fuse.Status { if fh == nil { // If handle is not found, it might have been already released // This is not an error condition for FLUSH - if in.LockOwner != 0 { - wfs.releasePosixOwner(in.NodeId, in.LockOwner) + if in.LockOwner != 0 && wfs.releasePosixOwner(in.NodeId, in.LockOwner) { + wfs.forgetLockOwner(in.NodeId, in.LockOwner) } return fuse.OK } @@ -89,8 +89,8 @@ func (wfs *WFS) Flush(cancel <-chan struct{}, in *fuse.FlushIn) fuse.Status { defer cancelFunc() status := wfs.doFlush(ctx, fh, in.Uid, in.Gid, allowAsync) - if in.LockOwner != 0 { - wfs.releasePosixOwner(in.NodeId, in.LockOwner) + if in.LockOwner != 0 && wfs.releasePosixOwner(in.NodeId, in.LockOwner) { + wfs.forgetLockOwner(in.NodeId, in.LockOwner) } return status } diff --git a/weed/mount/weedfs_filehandle.go b/weed/mount/weedfs_filehandle.go index c5d25e1cbf..e71a5fd03c 100644 --- a/weed/mount/weedfs_filehandle.go +++ b/weed/mount/weedfs_filehandle.go @@ -11,18 +11,20 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" ) -func (wfs *WFS) AcquireHandle(ctx context.Context, inode uint64, flags, uid, gid uint32) (fileHandle *FileHandle, status fuse.Status) { +// AcquireHandle returns the shared handle for inode plus this open's own fh +// value — the id handed to the kernel and seen again at RELEASE. +func (wfs *WFS) AcquireHandle(ctx context.Context, inode uint64, flags, uid, gid uint32) (fileHandle *FileHandle, openFh FileHandleId, status fuse.Status) { // If there is an in-flight async flush for this inode, wait for it to // complete before reopening. Otherwise the new handle would be built // from pre-close filer metadata and its next flush could overwrite the // data that was just written asynchronously. if !wfs.waitForPendingAsyncFlush(ctx, inode) { - return nil, fuse.EINTR + return nil, 0, fuse.EINTR } path, pathStatus := wfs.inodeToPath.GetPath(inode) if pathStatus != fuse.OK { - return nil, pathStatus + return nil, 0, pathStatus } // A fresh lookup returns the entry with the filer log position it @@ -43,12 +45,12 @@ func (wfs *WFS) AcquireHandle(ctx context.Context, inode uint64, flags, uid, gid } else { entry, entryVersionTsNs, status = wfs.maybeLoadEntry(ctx, path) if status != fuse.OK { - return nil, status + return nil, 0, status } freshLookup = true } if wormEnforced, _ := wfs.wormEnforcedForEntry(path, entry); wormEnforced && flags&fuse.O_ANYWRITE != 0 { - return nil, fuse.EPERM + return nil, 0, fuse.EPERM } // Check unix permission bits for the requested access mode. With // default_permissions the kernel already enforced them before this @@ -59,12 +61,12 @@ func (wfs *WFS) AcquireHandle(ctx context.Context, inode uint64, flags, uid, gid fileUid, fileGid = wfs.option.UidGidMapper.FilerToLocal(fileUid, fileGid) } if mask := openFlagsToAccessMask(flags); mask != 0 && !hasAccess(uid, gid, fileUid, fileGid, entry.Attributes.FileMode, mask) { - return nil, fuse.EACCES + return nil, 0, fuse.EACCES } } // need to AcquireFileHandle again to ensure correct handle counter var existed bool - fileHandle, existed = wfs.fhMap.AcquireFileHandle(wfs, inode, entry, entryVersionTsNs.tsNs, entryVersionTsNs.signature) + fileHandle, openFh, existed = wfs.fhMap.AcquireFileHandle(wfs, inode, entry, entryVersionTsNs.tsNs, entryVersionTsNs.signature) fileHandle.RememberPath(path) if existed && freshLookup { // Another opener created the handle while our lookup was in flight; @@ -84,12 +86,12 @@ func (wfs *WFS) AcquireHandle(ctx context.Context, inode uint64, flags, uid, gid if fileHandle.dlmLock == nil { // No lock server is reachable: proceeding would silently drop // cross-mount write serialization, so fail the open instead. - wfs.fhMap.ReleaseByHandle(fileHandle.fh) - return nil, fuse.Status(syscall.EAGAIN) + wfs.fhMap.ReleaseByHandle(openFh) + return nil, 0, fuse.Status(syscall.EAGAIN) } glog.V(1).Infof("DLM lock acquired for %s", path) } - return fileHandle, fuse.OK + return fileHandle, openFh, fuse.OK } // ReleaseHandle is called from FUSE Release. For handles with a pending diff --git a/weed/mount/weedfs_invalidate_open_handle_test.go b/weed/mount/weedfs_invalidate_open_handle_test.go index d652023684..752891249f 100644 --- a/weed/mount/weedfs_invalidate_open_handle_test.go +++ b/weed/mount/weedfs_invalidate_open_handle_test.go @@ -195,7 +195,7 @@ func TestUpdateEventRefreshesOpenFileHandle(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -260,7 +260,7 @@ func TestQueuedEventDoesNotRollBackNewerLocalState(t *testing.T) { wfs.inodeToPath.MarkChildrenCached(util.FullPath("/dir")) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -302,7 +302,7 @@ func TestBufferedBuildEventReinvalidatesOnCompletion(t *testing.T) { wfs.inodeToPath.Lookup(util.FullPath("/dir"), time.Now().Unix(), true, false, 0, false) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -352,7 +352,7 @@ func TestUncachedDirStaleStoreEntryDoesNotMaskEvent(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -388,7 +388,7 @@ func TestQueuedEventOlderThanFlushedStateIsIgnored(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -424,7 +424,7 @@ func TestSaveEntryKeepsOpenHandleAheadOfOlderEvents(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -462,7 +462,7 @@ func TestQueuedEventDoesNotRollBackHandleOpenedAfterEnqueue(t *testing.T) { // Stall the single invalidation worker on an unrelated handle's lock so // queued events outlive the open below. blockerInode := wfs.inodeToPath.Lookup(util.FullPath("/other/blocker"), time.Now().Unix(), false, false, 0, false) - blockerFh, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ + blockerFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ Name: "blocker", Attributes: &filer_pb.FuseAttributes{FileSize: 1}, }, 0, 0) @@ -499,7 +499,7 @@ func TestQueuedEventDoesNotRollBackHandleOpenedAfterEnqueue(t *testing.T) { // Open now: the lookup reaches the filer, which serves the newer // size-200 state versioned at log position 2000. inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("AcquireHandle status = %v, want OK", status) } @@ -523,7 +523,7 @@ func TestAbortedBuildEventDoesNotRollBackLaterOpen(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{lookupSize: 200, lookupLogTsNs: 2000}) blockerInode := wfs.inodeToPath.Lookup(util.FullPath("/other/blocker"), time.Now().Unix(), false, false, 0, false) - blockerFh, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ + blockerFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ Name: "blocker", Attributes: &filer_pb.FuseAttributes{FileSize: 1}, }, 0, 0) @@ -563,7 +563,7 @@ func TestAbortedBuildEventDoesNotRollBackLaterOpen(t *testing.T) { } inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("AcquireHandle status = %v, want OK", status) } @@ -590,7 +590,7 @@ func TestEventDuringOpenLookupIsFenced(t *testing.T) { startFakeFiler(t, wfs, fake) blockerInode := wfs.inodeToPath.Lookup(util.FullPath("/other/blocker"), time.Now().Unix(), false, false, 0, false) - blockerFh, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ + blockerFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, blockerInode, &filer_pb.Entry{ Name: "blocker", Attributes: &filer_pb.FuseAttributes{FileSize: 1}, }, 0, 0) @@ -626,7 +626,7 @@ func TestEventDuringOpenLookupIsFenced(t *testing.T) { } opened := make(chan openResult, 1) go func() { - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) opened <- openResult{fh, status} }() @@ -667,7 +667,7 @@ func TestRemoteCacheResponseVersionFencesUndeliveredEvents(t *testing.T) { } inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -699,7 +699,7 @@ func TestEventlessSaveAckStillFencesOlderEvents(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{updateEventless: true, updateLogTsNs: 2000}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -755,7 +755,7 @@ func TestLocalAckDoesNotFenceUnrelatedDelayedEvents(t *testing.T) { // Open the file: the cached-store read must not claim position 2000 — // the local ack versioned only its own path. inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("AcquireHandle status = %v, want OK", status) } @@ -794,14 +794,14 @@ func TestSlowerConcurrentOpenDoesNotOverwriteNewerHandle(t *testing.T) { } slow := make(chan openResult, 1) go func() { - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) slow <- openResult{fh, status} }() <-fake.lookupStarted // The faster opener completes with newer state while the slow lookup is // still in flight. - fastFh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fastFh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("fast AcquireHandle status = %v, want OK", status) } @@ -884,7 +884,7 @@ func TestAlreadyReflectedEventDoesNotDestroyDirtyPages(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -925,12 +925,12 @@ func TestSlowerOpenRejectedWhenHandleDirtyOrLookupUnversioned(t *testing.T) { } slow := make(chan openResult, 1) go func() { - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) slow <- openResult{fh, status} }() <-fake.lookupStarted - fastFh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fastFh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("fast AcquireHandle status = %v, want OK", status) } @@ -969,12 +969,12 @@ func TestUnversionedSlowerOpenDoesNotOverwrite(t *testing.T) { } slow := make(chan openResult, 1) go func() { - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) slow <- openResult{fh, status} }() <-fake.lookupStarted - fastFh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fastFh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("fast AcquireHandle status = %v, want OK", status) } @@ -998,7 +998,7 @@ func TestAlreadyReflectedEventPreservesDirtyWrites(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -1077,7 +1077,7 @@ func TestServerSideCopyInstallEnrollsInBase(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, }, 0, 0) @@ -1198,7 +1198,7 @@ func TestCommittedCopyWithFailedReadbackAdoptsItsEvent(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, }, 0, 0) @@ -1311,7 +1311,7 @@ func TestFlushAckCancelsPendingCopyEventAdoption(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, }, 0, 0) @@ -1356,7 +1356,7 @@ func TestAckedSaveInstallsIntoRacingOpenHandle(t *testing.T) { // The handle opened after the setattr path found none, before the ack. inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -1378,7 +1378,7 @@ func TestAckedSaveInstallsIntoRacingOpenHandle(t *testing.T) { // A dirty handle is left alone entirely: neither entry nor version. dirtyInode := wfs.inodeToPath.Lookup(util.FullPath("/dir/dirty"), time.Now().Unix(), false, false, 0, false) - dirtyFh, _ := wfs.fhMap.AcquireFileHandle(wfs, dirtyInode, &filer_pb.Entry{ + dirtyFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dirtyInode, &filer_pb.Entry{ Name: "dirty", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -1434,7 +1434,7 @@ func TestVacateEventPreservesDirtyPages(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -1469,7 +1469,7 @@ func TestCopyAdoptRejectsForeignEvent(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, }, 0, 0) @@ -1500,7 +1500,7 @@ func TestRemoteDownloadBaseMappedToLocal(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{cacheSize: 200, cacheUid: 2000, cacheLogTsNs: 1000}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 200}, @@ -1545,7 +1545,7 @@ func TestForeignDeleteMarksHandleDeleted(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -1603,7 +1603,7 @@ func TestStaleRemoteDownloadDoesNotRollBack(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{cacheSize: 100, cacheLogTsNs: 1000}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 200}, @@ -1637,7 +1637,7 @@ func TestRemoteOnlyHandleTakesUnversionedDownload(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{cacheSize: 100}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 100}, @@ -1662,7 +1662,7 @@ func TestCopyAdoptRejectsForeignMetadataChange(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200, FileMode: 0100644}, }, 0, 0) @@ -1697,7 +1697,7 @@ func TestForeignRenameDoesNotMarkHandleDeleted(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, }, 0, 0) @@ -1752,7 +1752,7 @@ func TestCopyAdoptAppliesForeignTouchToCleanHandle(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200, Mtime: 100}, }, 0, 0) @@ -1788,7 +1788,7 @@ func TestUnversionedRemoteDownloadDoesNotOverwriteVersioned(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{cacheSize: 100}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 200}, @@ -1820,7 +1820,7 @@ func TestEventFromAnotherFilerIsNotFencedByForeignClock(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{lookupSize: 200, lookupLogTsNs: 5000, lookupSignature: 11}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) + fh, _, status := wfs.AcquireHandle(context.Background(), inode, 0, 0, 0) if status != fuse.OK { t.Fatalf("AcquireHandle status = %v, want OK", status) } @@ -1860,7 +1860,7 @@ func TestTouchBeforeCopyEventKeepsPostCopyWrites(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 100, Mtime: 100}, }, 0, 0) @@ -1924,7 +1924,7 @@ func TestRejectedDownloadDoesNotRollBackCache(t *testing.T) { wfs.metaCache.WaitForEntryInvalidations() inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, Chunks: []*filer_pb.FileChunk{{FileId: "1,ab1", Size: 200}}, @@ -1959,7 +1959,7 @@ func TestRemoteOnlyHandleRefusesKnownOlderDownload(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{cacheSize: 100, cacheLogTsNs: 1000}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200}, RemoteEntry: &filer_pb.RemoteEntry{RemoteSize: 200}, @@ -1981,7 +1981,7 @@ func TestForeignChmodPreservesDirtyPages(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 200, FileMode: 0100644}, }, 0, 0) @@ -2013,12 +2013,12 @@ func TestRenameOverExistingMarksReplacedHandleDeleted(t *testing.T) { wfs := newInvalidateTestWFS(t) srcInode := wfs.inodeToPath.Lookup(util.FullPath("/dir/src"), time.Now().Unix(), false, false, 0, false) - srcFh, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ + srcFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, srcInode, &filer_pb.Entry{ Name: "src", Attributes: &filer_pb.FuseAttributes{FileSize: 100}, }, 0, 0) dstInode := wfs.inodeToPath.Lookup(util.FullPath("/dir/dst"), time.Now().Unix(), false, false, 0, false) - dstFh, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ + dstFh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, dstInode, &filer_pb.Entry{ Name: "dst", Attributes: &filer_pb.FuseAttributes{FileSize: 900}, }, 0, 0) @@ -2053,7 +2053,7 @@ func TestAckFromAnotherFilerIsNotDroppedBehindForeignFence(t *testing.T) { wfs := newInvalidateTestWFS(t) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) diff --git a/weed/mount/weedfs_posix_lock_routed.go b/weed/mount/weedfs_posix_lock_routed.go index ae7a48ce84..d2f32fa3d7 100644 --- a/weed/mount/weedfs_posix_lock_routed.go +++ b/weed/mount/weedfs_posix_lock_routed.go @@ -297,13 +297,13 @@ func posixPollAcquire(cancel <-chan struct{}, try func() (bool, error)) fuse.Sta } } -func (wfs *WFS) routedReleasePosixOwner(inode, owner uint64) { +func (wfs *WFS) routedReleasePosixOwner(inode, owner uint64) bool { if !wfs.posixHint.has(inode, owner) { - return + return true } key, ok := wfs.posixLockKeyForInode(inode) if !ok { - return + return false } wfs.posixOwn.ReleasePosixOwner(key, wfs.posixSid, owner) ctx, cancel := context.WithTimeout(context.Background(), posixLockReleaseTimeout) @@ -313,9 +313,10 @@ func (wfs *WFS) routedReleasePosixOwner(inode, owner uint64) { // transient failure would strand the lock until the owner filer's // session-lease reaping expires it. glog.Warningf("routed release posix owner %s: %v", key, err) - return + return false } wfs.posixHint.drop(inode, owner) + return true } func (wfs *WFS) routedReleaseFlockOwner(inode, owner uint64) { @@ -343,12 +344,16 @@ func (wfs *WFS) crossMountLocks() bool { // authority or the local table based on crossMountLocks, keeping the Flush and // Release call sites flag-agnostic. -func (wfs *WFS) releasePosixOwner(inode, owner uint64) { +// releasePosixOwner reports whether the owner's locks were released. A false +// return means a routed release failed and the close-time owner records must +// be kept so a later Flush or the final Release retries instead of stranding +// a live lock on the owner filer. +func (wfs *WFS) releasePosixOwner(inode, owner uint64) bool { if wfs.crossMountLocks() { - wfs.routedReleasePosixOwner(inode, owner) - return + return wfs.routedReleasePosixOwner(inode, owner) } wfs.posixLocks.ReleasePosixOwner(inode, owner) + return true } func (wfs *WFS) releaseFlockOwner(inode, owner uint64) { diff --git a/weed/mount/weedfs_release_flock_test.go b/weed/mount/weedfs_release_flock_test.go index 17c487c994..676f787988 100644 --- a/weed/mount/weedfs_release_flock_test.go +++ b/weed/mount/weedfs_release_flock_test.go @@ -20,7 +20,7 @@ func TestReleaseDropsFlockWhileFlushBlocked(t *testing.T) { startFakeFiler(t, wfs, &fakeFilerServer{}) inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/f"), time.Now().Unix(), false, false, 0, false) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "f", Attributes: &filer_pb.FuseAttributes{}, }, 0, 0) @@ -75,3 +75,65 @@ func TestReleaseDropsFlockWhileFlushBlocked(t *testing.T) { t.Fatal("release never completed after the flush unblocked") } } + +// OFD locks are scoped to the open file description, not the process, and the +// kernel reports no lock_owner for them at RELEASE — only flock holders carry +// one. Draining the owners recorded for the released handle frees them, so a +// restarted process (new owner) no longer sees the dead description's write +// lock. +func TestReleaseDropsOfdLocks(t *testing.T) { + wfs := newInvalidateTestWFS(t) + wfs.posixLocks = NewPosixLockTable() + startFakeFiler(t, wfs, &fakeFilerServer{}) + + inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/f"), time.Now().Unix(), false, false, 0, false) + entry := &filer_pb.Entry{ + Name: "f", + Attributes: &filer_pb.FuseAttributes{}, + } + _, openFh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, entry, 0, 0) + _, siblingFh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, entry, 0, 0) + if siblingFh == openFh { + t.Fatal("sibling open must get its own fh value") + } + + const ofdOwner = uint64(0x0fd0001) + ofd := &fuse.LkIn{ + InHeader: fuse.InHeader{NodeId: inode}, + Fh: uint64(openFh), + Owner: ofdOwner, + Lk: fuse.FileLock{Start: 0, End: math.MaxUint64, Typ: f_RDLCK}, + } + // A second open file description holds a read lock on the same inode; + // it must survive the first description's release. + const siblingOwner = uint64(0x0fd0002) + sibling := &fuse.LkIn{ + InHeader: fuse.InHeader{NodeId: inode}, + Fh: uint64(siblingFh), + Owner: siblingOwner, + Lk: fuse.FileLock{Start: 0, End: math.MaxUint64, Typ: f_RDLCK}, + } + if st := wfs.SetLk(nil, sibling); st != fuse.OK { + t.Fatalf("take sibling read lock: %v", st) + } + if st := wfs.SetLk(nil, ofd); st != fuse.OK { + t.Fatalf("take ofd lock: %v", st) + } + + wfs.Release(nil, &fuse.ReleaseIn{ + InHeader: fuse.InHeader{NodeId: inode}, + Fh: uint64(openFh), + }) + + // The dead description's read lock is gone: the sibling can upgrade to a + // write lock only because nothing else blocks it. + var out fuse.LkOut + wfs.posixLocks.GetLk(inode, lockRange{Start: 0, End: math.MaxUint64, Typ: f_WRLCK, Owner: 7}, &out) + if out.Lk.Typ != f_RDLCK { + t.Fatalf("sibling lock lost on release: typ=%d", out.Lk.Typ) + } + sibling.Lk.Typ = f_WRLCK + if st := wfs.SetLk(nil, sibling); st != fuse.OK { + t.Fatalf("ofd lock survived release: upgrade got %v", st) + } +} diff --git a/weed/mount/weedfs_remote_rename_test.go b/weed/mount/weedfs_remote_rename_test.go index ba04f0500c..08a586cb6a 100644 --- a/weed/mount/weedfs_remote_rename_test.go +++ b/weed/mount/weedfs_remote_rename_test.go @@ -71,7 +71,7 @@ func TestRemoteRenameReplayLeavesTheTargetAlone(t *testing.T) { oldPath, newPath := util.FullPath("/dir/file"), util.FullPath("/dir/renamed") inode := wfs.inodeToPath.Lookup(oldPath, time.Now().Unix(), false, false, 0, true) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) @@ -100,7 +100,7 @@ func TestRenameBelowTheVersionFenceStillMoves(t *testing.T) { oldPath, newPath := util.FullPath("/dir/file"), util.FullPath("/dir/renamed") inode := wfs.inodeToPath.Lookup(oldPath, time.Now().Unix(), false, false, 0, true) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "file", Attributes: &filer_pb.FuseAttributes{FileSize: 88}, }, 0, 0) diff --git a/weed/mount/weedfs_xattr_test.go b/weed/mount/weedfs_xattr_test.go index 9b2e1b49b8..51ffb1cd8e 100644 --- a/weed/mount/weedfs_xattr_test.go +++ b/weed/mount/weedfs_xattr_test.go @@ -26,7 +26,7 @@ func TestSetXAttrCopiesValueBuffer(t *testing.T) { path := util.FullPath("/aaa.txt") inode := wfs.inodeToPath.Lookup(path, 1, false, false, 0, true) - fh, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ + fh, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, &filer_pb.Entry{ Name: "aaa.txt", Attributes: &filer_pb.FuseAttributes{ FileMode: 0100644,