From 0ea818a44f7c62d50163819ed972cd2adf6c35bf Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 26 Oct 2026 22:02:13 -0700 Subject: [PATCH] mount: drain OFD locks when a file description is released (#12117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * mount: drain OFD locks when a file description is released The kernel scopes OFD fcntl locks to the open file description but reports no lock_owner for them at RELEASE — it only sends one for flock holders — so an OFD write lock taken on a file handle outlived the holder and blocked every later open of that file (e.g. etcd failing to reopen its WAL after a restart). The mount now records the owner of each granted non-flock lock under its file handle and releases those owners on RELEASE, which also covers any POSIX locks left over when FLUSH could not run. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: give every open its own fh value over the shared handle AcquireFileHandle reuses one FileHandle per inode, so every open reported the same fh to the kernel. A RELEASE for one description could then not be told apart from a sibling still holding the inode open — draining lock owners recorded under the shared fh dropped locks belonging to descriptions that stay open. Each open now registers its own fh value in fh2inode, all resolving to the shared FileHandle, and RELEASE removes exactly the id that is closing. This also stops a duplicated RELEASE on one id from double-decrementing the open count. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: forget flushed lock owners so a later release cannot re-drop locks POSIX lock owners are process-scoped: Flush releases every lock the process holds on the inode, but the owner stayed recorded under each open's fh for the Release-time drain. If the process took a fresh lock through another open before an old description finished closing, the stale record made that RELEASE drop the new lock too. Flush now drops the released owner from every recorded open of the inode, so the Release drain only ever fires for owners whose locks were never flushed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: keep close-time lock owner records when the release fails A routed POSIX owner release that errors keeps the flush hint for a retry, but Flush also dropped the owner from every open's release records unconditionally — so the final Release skipped the retry and the owner filer's lock was stranded until lease reaping. Only forget the owner once its locks were actually released. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/mount/filehandle.go | 2 + weed/mount/filehandle_map.go | 43 ++++++++- weed/mount/filehandle_read_remote_test.go | 2 +- weed/mount/posix_file_lock.go | 47 ++++++++++ weed/mount/weedfs.go | 1 + weed/mount/weedfs_file_copy_range_test.go | 16 ++-- weed/mount/weedfs_file_fallocate_test.go | 2 +- weed/mount/weedfs_file_io.go | 16 +++- weed/mount/weedfs_file_lock.go | 76 ++++++++++----- weed/mount/weedfs_file_mkrm.go | 16 ++-- weed/mount/weedfs_file_mkrm_test.go | 6 +- weed/mount/weedfs_file_sync.go | 8 +- weed/mount/weedfs_filehandle.go | 22 +++-- .../weedfs_invalidate_open_handle_test.go | 92 +++++++++---------- weed/mount/weedfs_posix_lock_routed.go | 19 ++-- weed/mount/weedfs_release_flock_test.go | 64 ++++++++++++- weed/mount/weedfs_remote_rename_test.go | 4 +- weed/mount/weedfs_xattr_test.go | 2 +- 18 files changed, 313 insertions(+), 125 deletions(-) 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,