diff --git a/weed/mount/inode_to_path.go b/weed/mount/inode_to_path.go index 6f0bc8890..1f2ca2d70 100644 --- a/weed/mount/inode_to_path.go +++ b/weed/mount/inode_to_path.go @@ -483,7 +483,12 @@ func (i *InodeToPath) AddPath(inode uint64, path util.FullPath) { } } -func (i *InodeToPath) RemovePath(path util.FullPath) { +// RemovePath drops the name. onStillReferenced, if given, runs under the +// table's lock when the kernel still holds lookup references to the inode: +// such an inode keeps receiving requests until the final forget, and holding +// the lock is what keeps that forget from racing whatever per-inode state the +// callback installs — Forget releases under the same lock. +func (i *InodeToPath) RemovePath(path util.FullPath, onStillReferenced func(inode uint64)) { i.Lock() defer i.Unlock() inode, found := i.path2inode[path] @@ -491,6 +496,9 @@ func (i *InodeToPath) RemovePath(path util.FullPath) { delete(i.path2inode, path) i.dropDirPath(inode) i.removePathFromInode2Path(inode, path) + if ie := i.inode2path[inode]; ie != nil && ie.nlookup > 0 && onStillReferenced != nil { + onStillReferenced(inode) + } } } diff --git a/weed/mount/inode_to_path_dirstate_test.go b/weed/mount/inode_to_path_dirstate_test.go index dc75e6847..66c63016a 100644 --- a/weed/mount/inode_to_path_dirstate_test.go +++ b/weed/mount/inode_to_path_dirstate_test.go @@ -56,7 +56,7 @@ func TestDirStateIndexesStayInStep(t *testing.T) { t.Error("renamed directory not indexed") } - itp.RemovePath("/a/moved") + itp.RemovePath("/a/moved", nil) if itp.dirStateOf("/a/moved") != nil { t.Error("removed directory still indexed") } @@ -77,7 +77,7 @@ func TestForgetDoesNotDropAReusedDirPath(t *testing.T) { now := time.Now().Unix() oldInode := itp.Lookup("/a", now, true, false, 0, true) - itp.RemovePath("/a") + itp.RemovePath("/a", nil) newInode := itp.Lookup("/a", now+1, true, false, 0, true) if newInode == oldInode { t.Fatalf("recreated directory reused inode %d", newInode) diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index 79f897c6a..4039c5ba3 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -177,8 +177,10 @@ type WFS struct { atimeMap map[uint64]time.Time // inode -> atime, in-memory only, bounded dirMtimeMu sync.Mutex dirMtimeMap map[uint64]time.Time // inode -> mtime/ctime, in-memory overlay for dirs - entryValidSec uint64 // kernel FUSE entry cache TTL in seconds - attrValidSec uint64 // kernel FUSE attr cache TTL in seconds + removedDirMu sync.Mutex + removedDirs map[uint64]*filer_pb.Entry // inode -> last-known entry of a directory removed while still referenced + entryValidSec uint64 // kernel FUSE entry cache TTL in seconds + attrValidSec uint64 // kernel FUSE attr cache TTL in seconds dirIdleEvict time.Duration // openMtimeCache maps inode -> [mtime_sec, mtime_ns] from the last Open. @@ -524,7 +526,8 @@ func (wfs *WFS) Init(server *fuse.Server) { // maybeReadEntry resolves an inode to the entry metadata operations act on. An // open handle answers ahead of the path: unlink drops the name while the // descriptor stays valid, so an unlinked-but-open file returns its handle's -// entry with an empty path instead of ENOENT. +// entry with an empty path instead of ENOENT. A removed directory has no +// handle; its remembered entry answers the same way. func (wfs *WFS) maybeReadEntry(inode uint64) (path util.FullPath, fh *FileHandle, entry *filer_pb.Entry, status fuse.Status) { var found bool if fh, found = wfs.fhMap.FindFileHandle(inode); found { @@ -538,6 +541,9 @@ func (wfs *WFS) maybeReadEntry(inode uint64) (path util.FullPath, fh *FileHandle } path, status = wfs.inodeToPath.GetPath(inode) if status != fuse.OK { + if entry = wfs.removedDirEntry(inode); entry != nil { + return "", nil, entry, fuse.OK + } return } entry, _, status = wfs.maybeLoadEntry(path) diff --git a/weed/mount/weedfs_attr.go b/weed/mount/weedfs_attr.go index 3922201bb..39c34e35c 100644 --- a/weed/mount/weedfs_attr.go +++ b/weed/mount/weedfs_attr.go @@ -7,6 +7,8 @@ import ( "time" "github.com/seaweedfs/go-fuse/v2/fuse" + "google.golang.org/protobuf/proto" + "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" @@ -206,6 +208,11 @@ func (wfs *WFS) SetAttr(cancel <-chan struct{}, input *fuse.SetAttrIn, out *fuse fh.dirtyMetadata = true return fuse.OK } + if path == "" { + // removed while open: the remembered entry is all there is to update + wfs.rememberRemovedDir(input.NodeId, entry) + return fuse.OK + } return wfs.saveEntry(path, entry) @@ -399,10 +406,10 @@ func (wfs *WFS) setAtime(inode uint64, t time.Time) { } // applyInMemoryAtime overlays the in-memory atime onto a fuse.Attr if present. -// forgetInMemoryTimes drops the overlays for an inode. Both maps are keyed by -// inode and inodes are derived from the path, so a delete and recreate can +// forgetInMemoryTimes drops the overlays for an inode. All these maps are keyed +// by inode and inodes are derived from the path, so a delete and recreate can // hand the same number to a different file — which would then inherit the -// previous one's access or modification time. +// previous one's access or modification time, or a removed directory's entry. func (wfs *WFS) forgetInMemoryTimes(inode uint64) { wfs.atimeMu.Lock() delete(wfs.atimeMap, inode) @@ -411,6 +418,41 @@ func (wfs *WFS) forgetInMemoryTimes(inode uint64) { wfs.dirMtimeMu.Lock() delete(wfs.dirMtimeMap, inode) wfs.dirMtimeMu.Unlock() + + wfs.removedDirMu.Lock() + delete(wfs.removedDirs, inode) + wfs.removedDirMu.Unlock() +} + +// rememberRemovedDir keeps the last-known entry of a directory removed while +// the kernel still references its inode. A directory has no file handle to +// live on through, so this is what serves fstat/fchmod/f*xattr on a still-open +// descriptor until the final forget. Mutations publish through here as well, +// replacing the stored entry wholesale. +// +// The final forget's cleanup cannot miss an insert: Rmdir inserts inside +// RemovePath's callback, under the same inode table lock the forget releases +// under, and a publish runs inside a request whose open descriptor keeps the +// kernel from issuing that forget at all. +func (wfs *WFS) rememberRemovedDir(inode uint64, entry *filer_pb.Entry) { + wfs.removedDirMu.Lock() + if wfs.removedDirs == nil { + wfs.removedDirs = make(map[uint64]*filer_pb.Entry) + } + wfs.removedDirs[inode] = entry + wfs.removedDirMu.Unlock() +} + +// removedDirEntry hands out a private copy: stored entries are never mutated +// in place, so concurrent readers cannot see a half-applied change. +func (wfs *WFS) removedDirEntry(inode uint64) *filer_pb.Entry { + wfs.removedDirMu.Lock() + entry := wfs.removedDirs[inode] + wfs.removedDirMu.Unlock() + if entry == nil { + return nil + } + return proto.Clone(entry).(*filer_pb.Entry) } func (wfs *WFS) applyInMemoryAtime(out *fuse.Attr, inode uint64) { diff --git a/weed/mount/weedfs_attr_unlinked_test.go b/weed/mount/weedfs_attr_unlinked_test.go index 619431515..1da8885e5 100644 --- a/weed/mount/weedfs_attr_unlinked_test.go +++ b/weed/mount/weedfs_attr_unlinked_test.go @@ -1,7 +1,9 @@ package mount import ( + "os" "testing" + "time" "github.com/seaweedfs/go-fuse/v2/fuse" "github.com/seaweedfs/seaweedfs/weed/filer" @@ -47,7 +49,7 @@ func newUnlinkedOpenFile(t *testing.T) (*WFS, uint64, *FileHandle) { wfs.fhMap.inode2fh[inode] = fh wfs.fhMap.fh2inode[fh.fh] = inode - wfs.inodeToPath.RemovePath(fullPath) + wfs.inodeToPath.RemovePath(fullPath, nil) fh.isDeleted = true return wfs, inode, fh @@ -138,3 +140,153 @@ func TestXAttrOnUnlinkedOpenFile(t *testing.T) { t.Fatalf("RemoveXAttr on unlinked open file: got %v, want OK", status) } } + +// newRemovedOpenDir builds a WFS holding one kernel-referenced directory whose +// name rmdir has already dropped, the state between rmdir() and releasedir(). +func newRemovedOpenDir(t *testing.T) (*WFS, uint64) { + t.Helper() + + wfs := &WFS{ + option: &Option{}, + inodeToPath: NewInodeToPath(util.FullPath("/"), 0), + fhMap: NewFileHandleToInode(), + fhLockTable: util.NewLockTable[FileHandleId](), + atimeMap: make(map[uint64]time.Time, 8), + dirMtimeMap: make(map[uint64]time.Time, 8), + } + + const inode = uint64(7) + fullPath := util.FullPath("/dir") + wfs.inodeToPath.Lookup(fullPath, 1, true, false, inode, true) + + kept := false + wfs.inodeToPath.RemovePath(fullPath, func(removedInode uint64) { + kept = true + wfs.rememberRemovedDir(removedInode, &filer_pb.Entry{ + Name: "dir", + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{FileMode: uint32(os.ModeDir | 0755), Uid: 1, Gid: 2}, + }) + }) + if !kept { + t.Fatal("RemovePath did not report the inode as still referenced") + } + + return wfs, inode +} + +// TestSetAttrOnRemovedOpenDir covers fchmod/futimens on a descriptor whose +// directory has been removed: the inode lives on until the final forget, so +// these must not fail with ENOENT, and the change must be visible to a +// following fstat. +func TestSetAttrOnRemovedOpenDir(t *testing.T) { + wfs, inode := newRemovedOpenDir(t) + + in := &fuse.SetAttrIn{} + in.NodeId = inode + in.Valid = fuse.FATTR_MODE + in.Mode = 0770 + var out fuse.AttrOut + if status := wfs.SetAttr(nil, in, &out); status != fuse.OK { + t.Fatalf("SetAttr mode on removed open dir: got %v, want OK", status) + } + if out.Attr.Mode&0777 != 0770 { + t.Fatalf("SetAttr mode: got %o, want 770", out.Attr.Mode&0777) + } + if out.Attr.Nlink != 0 { + t.Fatalf("SetAttr nlink: got %d, want 0", out.Attr.Nlink) + } + + in = &fuse.SetAttrIn{} + in.NodeId = inode + in.Valid = fuse.FATTR_MTIME + in.Mtime = 12345 + if status := wfs.SetAttr(nil, in, &out); status != fuse.OK { + t.Fatalf("SetAttr mtime on removed open dir: got %v, want OK", status) + } + + gin := &fuse.GetAttrIn{} + gin.NodeId = inode + if status := wfs.GetAttr(nil, gin, &out); status != fuse.OK { + t.Fatalf("GetAttr on removed open dir: got %v, want OK", status) + } + if out.Attr.Mode&0777 != 0770 { + t.Fatalf("GetAttr mode after chmod: got %o, want 770", out.Attr.Mode&0777) + } + if out.Attr.Mode&fuse.S_IFDIR == 0 { + t.Fatalf("GetAttr lost the directory type: %o", out.Attr.Mode) + } + if out.Attr.Mtime != 12345 { + t.Fatalf("GetAttr mtime after utimens: got %d, want 12345", out.Attr.Mtime) + } + if out.Attr.Uid != 1 || out.Attr.Gid != 2 { + t.Fatalf("GetAttr uid/gid: got %d/%d, want 1/2", out.Attr.Uid, out.Attr.Gid) + } + if out.Attr.Nlink != 0 { + t.Fatalf("GetAttr nlink: got %d, want 0", out.Attr.Nlink) + } +} + +// TestXAttrOnRemovedOpenDir covers fsetxattr/fgetxattr/fremovexattr on a +// descriptor whose directory has been removed. +func TestXAttrOnRemovedOpenDir(t *testing.T) { + wfs, inode := newRemovedOpenDir(t) + + setIn := &fuse.SetXAttrIn{} + setIn.NodeId = inode + if status := wfs.SetXAttr(nil, setIn, "user.k", []byte("v")); status != fuse.OK { + t.Fatalf("SetXAttr on removed open dir: got %v, want OK", status) + } + + header := &fuse.InHeader{NodeId: inode} + dest := make([]byte, 8) + n, status := wfs.GetXAttr(nil, header, "user.k", dest) + if status != fuse.OK { + t.Fatalf("GetXAttr on removed open dir: got %v, want OK", status) + } + if string(dest[:n]) != "v" { + t.Fatalf("GetXAttr value: got %q, want %q", dest[:n], "v") + } + + if status := wfs.RemoveXAttr(nil, header, "user.k"); status != fuse.OK { + t.Fatalf("RemoveXAttr on removed open dir: got %v, want OK", status) + } + if _, status := wfs.GetXAttr(nil, header, "user.k", dest); status != fuse.ENOATTR { + t.Fatalf("GetXAttr after remove: got %v, want ENOATTR", status) + } +} + +// TestForgetReleasesRemovedOpenDir pins the cleanup: once the kernel drops its +// last reference, the remembered entry goes with it. +func TestForgetReleasesRemovedOpenDir(t *testing.T) { + wfs, inode := newRemovedOpenDir(t) + + wfs.Forget(inode, 1) + + in := &fuse.SetAttrIn{} + in.NodeId = inode + in.Valid = fuse.FATTR_MODE + in.Mode = 0700 + var out fuse.AttrOut + if status := wfs.SetAttr(nil, in, &out); status != fuse.ENOENT { + t.Fatalf("SetAttr after forget: got %v, want ENOENT", status) + } + if len(wfs.removedDirs) != 0 { + t.Fatalf("removedDirs not cleaned up: %d entries", len(wfs.removedDirs)) + } +} + +// TestRemovePathSkipsUnreferencedInode pins the insert-forget ordering: once +// the kernel's references are gone, RemovePath must not offer the inode for +// retention, so nothing can be inserted that the forget's cleanup missed. +func TestRemovePathSkipsUnreferencedInode(t *testing.T) { + itp := NewInodeToPath(util.FullPath("/"), 0) + fullPath := util.FullPath("/dir") + itp.Lookup(fullPath, 1, true, false, 7, true) + itp.Forget(7, 1, nil, nil) + + itp.Lookup(fullPath, 1, true, false, 7, false) + itp.RemovePath(fullPath, func(inode uint64) { + t.Fatalf("RemovePath offered unreferenced inode %d for retention", inode) + }) +} diff --git a/weed/mount/weedfs_dir_mkrm.go b/weed/mount/weedfs_dir_mkrm.go index 26ba969d2..43aa2cbb5 100644 --- a/weed/mount/weedfs_dir_mkrm.go +++ b/weed/mount/weedfs_dir_mkrm.go @@ -7,6 +7,7 @@ import ( "time" "github.com/seaweedfs/go-fuse/v2/fuse" + "google.golang.org/protobuf/proto" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/glog" @@ -138,10 +139,15 @@ func (wfs *WFS) Rmdir(cancel <-chan struct{}, header *fuse.InHeader, name string } entryFullPath := dirFullPath.Child(name) + targetEntry, _, targetCode := wfs.maybeLoadEntry(entryFullPath) + if targetCode != fuse.OK { + targetEntry = nil + } + // POSIX: enforce sticky bit on the parent directory. if dirEntry, _, dirCode := wfs.maybeLoadEntry(dirFullPath); dirCode == fuse.OK && dirEntry != nil && dirEntry.Attributes != nil { targetUid := uint32(0) - if targetEntry, _, targetCode := wfs.maybeLoadEntry(entryFullPath); targetCode == fuse.OK && targetEntry != nil && targetEntry.Attributes != nil { + if targetEntry != nil && targetEntry.Attributes != nil { targetUid = targetEntry.Attributes.Uid } if code := checkStickyBit(dirEntry.Attributes.FileMode, dirEntry.Attributes.Uid, targetUid, header.Uid); code != fuse.OK { @@ -174,7 +180,17 @@ func (wfs *WFS) Rmdir(cancel <-chan struct{}, header *fuse.InHeader, name string glog.Warningf("rmdir %s: best-effort metadata apply failed: %v", entryFullPath, applyErr) wfs.inodeToPath.InvalidateChildrenCache(dirFullPath) } - wfs.inodeToPath.RemovePath(entryFullPath) + // The filer serialized the delete against concurrent updates and returned + // the entry as it stood; the snapshot loaded above may predate one. + if oldEntry := resp.GetMetadataEvent().GetEventNotification().GetOldEntry(); oldEntry.GetAttributes() != nil { + targetEntry = proto.Clone(oldEntry).(*filer_pb.Entry) + wfs.mapPbIdFromFilerToLocal(targetEntry) + } + wfs.inodeToPath.RemovePath(entryFullPath, func(inode uint64) { + if targetEntry != nil { + wfs.rememberRemovedDir(inode, targetEntry) + } + }) wfs.inodeToPath.TouchDirectory(dirFullPath) wfs.touchDirMtimeCtimeBest(dirFullPath) wfs.inodeToPath.AdjustSubdirCount(dirFullPath, -1) diff --git a/weed/mount/weedfs_file_mkrm.go b/weed/mount/weedfs_file_mkrm.go index d00f479d4..7db012365 100644 --- a/weed/mount/weedfs_file_mkrm.go +++ b/weed/mount/weedfs_file_mkrm.go @@ -302,7 +302,7 @@ func (wfs *WFS) Unlink(cancel <-chan struct{}, header *fuse.InHeader, name strin } } - wfs.inodeToPath.RemovePath(entryFullPath) + wfs.inodeToPath.RemovePath(entryFullPath, nil) if isHardLink && sharedInode != 0 { decremented := proto.Clone(entry).(*filer_pb.Entry) diff --git a/weed/mount/weedfs_xattr.go b/weed/mount/weedfs_xattr.go index a042c0e00..07ee81032 100644 --- a/weed/mount/weedfs_xattr.go +++ b/weed/mount/weedfs_xattr.go @@ -141,6 +141,11 @@ func (wfs *WFS) SetXAttr(cancel <-chan struct{}, input *fuse.SetXAttrIn, attr st fh.dirtyMetadata = true return fuse.OK } + if path == "" { + // removed while open: the remembered entry is all there is to update + wfs.rememberRemovedDir(input.NodeId, entry) + return fuse.OK + } return wfs.saveEntry(path, entry) @@ -219,6 +224,11 @@ func (wfs *WFS) RemoveXAttr(cancel <-chan struct{}, header *fuse.InHeader, attr fh.dirtyMetadata = true return fuse.OK } + if path == "" { + // removed while open: the remembered entry is all there is to update + wfs.rememberRemovedDir(header.NodeId, entry) + return fuse.OK + } return wfs.saveEntry(path, entry) }