mount: keep metadata operations working on an unlinked open file (#10989)

* mount: serve metadata ops from the open handle of an unlinked file ftruncate on a descriptor whose file was unlinked failed with ENOENT: maybeReadEntry resolved the inode to a path first, and unlink had already dropped it. GetAttr worked around that with its own handle fallback; SetAttr and the xattr handlers had none. Look the handle up first and let it answer whether or not a name still points at the inode. GetAttr keeps reporting nlink 0 there, now off the empty path. Claude-Session: https://claude.ai/code/session_01U1R8BM4bVT46KwPDEj2Ega * mount: read an open handle's attributes under the handle lock too GetAttr held only the LockedEntry lock, which covers the async uploader's chunk appends but not Write or the metadata flush: those rewrite size, times and the whole chunk slice under the handle lock, so FileSize could walk a slice mid-reassignment. The branch this replaced took both locks; take both here, outer handle lock first, as Read and Lseek do. Claude-Session: https://claude.ai/code/session_01U1R8BM4bVT46KwPDEj2Ega * mount: report nlink 0 from SetAttr for an unlinked open file The kernel caches the attributes a SETATTR reply carries, so an ftruncate on an unlinked file left fstat reporting nlink 1 until the cache expired, even though GetAttr had it right. Both replies go through the same rule. Claude-Session: https://claude.ai/code/session_01U1R8BM4bVT46KwPDEj2Ega
2026-08-28 20:06:14 +00:00 · 2026-08-27 16:22:25 -07:00
parent e9a464840c
commit bc06505b40
4 changed files with 186 additions and 39 deletions
@@ -521,21 +521,26 @@ func (wfs *WFS) Init(server *fuse.Server) {
 	wfs.fuseServer = 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.
 func (wfs *WFS) maybeReadEntry(inode uint64) (path util.FullPath, fh *FileHandle, entry *filer_pb.Entry, status fuse.Status) {
-	path, status = wfs.inodeToPath.GetPath(inode)
-	if status != fuse.OK {
-		return
-	}
 	var found bool
 	if fh, found = wfs.fhMap.FindFileHandle(inode); found {
+		path, _ = wfs.inodeToPath.GetPath(inode)
 		entry = fh.UpdateEntry(func(entry *filer_pb.Entry) {
 			if entry != nil && fh.entry.Attributes == nil {
 				entry.Attributes = &filer_pb.FuseAttributes{}
 			}
 		})
-	} else {
-		entry, _, status = wfs.maybeLoadEntry(path)
+		return path, fh, entry, fuse.OK
 	}
+	path, status = wfs.inodeToPath.GetPath(inode)
+	if status != fuse.OK {
+		return
+	}
+	entry, _, status = wfs.maybeLoadEntry(path)
 	return
 }

@@ -25,43 +25,34 @@ func (wfs *WFS) GetAttr(cancel <-chan struct{}, input *fuse.GetAttrIn, out *fuse

 	inode := input.NodeId
 	path, fh, entry, status := wfs.maybeReadEntry(inode)
-	if status == fuse.OK {
-		out.AttrValid = wfs.attrValidSec
-		// When an open handle owns the entry, async upload workers append
-		// chunks under the LockedEntry lock; take it for reading so FileSize
-		// does not iterate the chunk slice mid-reallocation. Re-read under the
-		// lock in case SetEntry swapped the pointer since maybeReadEntry.
-		if fh != nil {
-			fh.entry.RLock()
-			entry = fh.entry.Entry
-		}
-		wfs.setAttrByPbEntry(&out.Attr, inode, entry, true)
-		if fh != nil {
-			fh.entry.RUnlock()
-		}
-		wfs.applyInMemoryAtime(&out.Attr, inode)
-		if entry.IsDirectory {
-			wfs.applyInMemoryDirMtime(&out.Attr, inode)
-			if wfs.option.PosixDirNlink {
-				wfs.applyDirNlink(&out.Attr, path)
-			}
-		}
+	if status != fuse.OK {
 		return status
+	}
+	out.AttrValid = wfs.attrValidSec
+	// An open handle's entry has two sets of writers: async upload workers
+	// append chunks under the LockedEntry lock, while Write and the metadata
+	// flush rewrite size, times and the whole chunk slice under the handle
+	// lock. Hold both for reading, in that order, so FileSize never iterates a
+	// slice mid-reallocation. Re-read the entry under them in case SetEntry
+	// swapped the pointer since maybeReadEntry.
+	if fh != nil {
+		fhActiveLock := wfs.fhLockTable.AcquireLock("GetAttr", fh.fh, util.SharedLock)
+		fh.entry.RLock()
+		entry = fh.entry.Entry
+		wfs.setAttrByPbEntry(&out.Attr, inode, entry, true)
+		fh.entry.RUnlock()
+		wfs.fhLockTable.ReleaseLock(fh.fh, fhActiveLock)
 	} else {
-		if fh, found := wfs.fhMap.FindFileHandle(inode); found {
-			out.AttrValid = wfs.attrValidSec
-			// Use shared lock to prevent race with Write operations
-			fhActiveLock := wfs.fhLockTable.AcquireLock("GetAttr", fh.fh, util.SharedLock)
-			fh.entry.RLock()
-			wfs.setAttrByPbEntry(&out.Attr, inode, fh.entry.Entry, true)
-			fh.entry.RUnlock()
-			wfs.fhLockTable.ReleaseLock(fh.fh, fhActiveLock)
-			wfs.applyInMemoryAtime(&out.Attr, inode)
-			out.Nlink = 0
-			return fuse.OK
+		wfs.setAttrByPbEntry(&out.Attr, inode, entry, true)
+	}
+	wfs.applyInMemoryAtime(&out.Attr, inode)
+	applyUnlinkedNlink(&out.Attr, path)
+	if entry.GetIsDirectory() {
+		wfs.applyInMemoryDirMtime(&out.Attr, inode)
+		if wfs.option.PosixDirNlink {
+			wfs.applyDirNlink(&out.Attr, path)
 		}
 	}
-
 	return status
 }

@@ -202,6 +193,7 @@ func (wfs *WFS) SetAttr(cancel <-chan struct{}, input *fuse.SetAttrIn, out *fuse
 	}
 	wfs.setAttrByPbEntry(&out.Attr, input.NodeId, entry, !includeSize)
 	wfs.applyInMemoryAtime(&out.Attr, input.NodeId)
+	applyUnlinkedNlink(&out.Attr, path)

 	if fh != nil {
 		fh.dirtyMetadata = true
@@ -423,6 +415,15 @@ func (wfs *WFS) applyInMemoryAtime(out *fuse.Attr, inode uint64) {
 	wfs.atimeMu.Unlock()
 }

+// applyUnlinkedNlink zeroes nlink for an inode no name points at any more: the
+// file was unlinked while open and lives on only through its handle. Every
+// reply carrying attributes must say so, or the kernel caches a live nlink.
+func applyUnlinkedNlink(out *fuse.Attr, path util.FullPath) {
+	if path == "" {
+		out.Nlink = 0
+	}
+}
+
 // applyDirNlink sets nlink = 2 + number_of_subdirectories for a directory.
 // Uses the in-memory subdirectory count tracked by mkdir/rmdir/rename.
 func (wfs *WFS) applyDirNlink(out *fuse.Attr, dirPath util.FullPath) {
@@ -23,6 +23,7 @@ func TestAttrChunkRace(t *testing.T) {
 		option:         &Option{},
 		inodeToPath:    NewInodeToPath(util.FullPath("/"), 0),
 		fhMap:          NewFileHandleToInode(),
+		fhLockTable:    util.NewLockTable[FileHandleId](),
 		openMtimeCache: make(map[uint64][2]int64, 8),
 	}

@@ -0,0 +1,140 @@
+package mount
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/go-fuse/v2/fuse"
+	"github.com/seaweedfs/seaweedfs/weed/filer"
+	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+)
+
+// newUnlinkedOpenFile builds a WFS holding one open handle whose name has
+// already been removed, the state a file is in between unlink() and the last
+// close() of a descriptor still pointing at it.
+func newUnlinkedOpenFile(t *testing.T) (*WFS, uint64, *FileHandle) {
+	t.Helper()
+
+	wfs := &WFS{
+		option:         &Option{ChunkSizeLimit: 1024, ConcurrentReaders: 1},
+		inodeToPath:    NewInodeToPath(util.FullPath("/"), 0),
+		fhMap:          NewFileHandleToInode(),
+		fhLockTable:    util.NewLockTable[FileHandleId](),
+		openMtimeCache: make(map[uint64][2]int64, 8),
+	}
+
+	const inode = uint64(42)
+	fullPath := util.FullPath("/dir/file")
+	wfs.inodeToPath.Lookup(fullPath, 1, false, false, inode, true)
+
+	entry := &filer_pb.Entry{
+		Name:       "file",
+		Attributes: &filer_pb.FuseAttributes{FileMode: 0644},
+	}
+	chunkGroup, err := filer.NewChunkGroup(nil, nil, nil, 1, nil)
+	if err != nil {
+		t.Fatalf("NewChunkGroup: %v", err)
+	}
+	fh := &FileHandle{
+		fh:              FileHandleId(1),
+		inode:           inode,
+		wfs:             wfs,
+		entry:           &LockedEntry{Entry: entry},
+		entryChunkGroup: chunkGroup,
+	}
+	fh.dirtyPages = newPageWriter(fh, 1<<20)
+	fh.RememberPath(fullPath)
+	wfs.fhMap.inode2fh[inode] = fh
+	wfs.fhMap.fh2inode[fh.fh] = inode
+
+	wfs.inodeToPath.RemovePath(fullPath)
+	fh.isDeleted = true
+
+	return wfs, inode, fh
+}
+
+// TestSetAttrOnUnlinkedOpenFile covers ftruncate/fchmod on a descriptor whose
+// file has been unlinked: POSIX keeps the open file alive, so these must not
+// fail with ENOENT.
+func TestSetAttrOnUnlinkedOpenFile(t *testing.T) {
+	wfs, inode, fh := newUnlinkedOpenFile(t)
+
+	in := &fuse.SetAttrIn{}
+	in.NodeId = inode
+	in.Valid = fuse.FATTR_SIZE
+	in.Size = 128
+	var out fuse.AttrOut
+	if status := wfs.SetAttr(nil, in, &out); status != fuse.OK {
+		t.Fatalf("SetAttr size on unlinked open file: got %v, want OK", status)
+	}
+	if out.Attr.Size != 128 {
+		t.Fatalf("SetAttr size: got %d, want 128", out.Attr.Size)
+	}
+	if got := fh.GetEntry().GetEntry().Attributes.FileSize; got != 128 {
+		t.Fatalf("handle entry FileSize: got %d, want 128", got)
+	}
+	if !fh.dirtyMetadata {
+		t.Fatal("SetAttr did not mark the handle dirty")
+	}
+	// The kernel caches what this reply carries, so it has to agree with fstat.
+	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_MODE
+	in.Mode = 0600
+	if status := wfs.SetAttr(nil, in, &out); status != fuse.OK {
+		t.Fatalf("SetAttr mode on unlinked open file: got %v, want OK", status)
+	}
+	if got := fh.GetEntry().GetEntry().Attributes.FileMode & 0777; got != 0600 {
+		t.Fatalf("handle entry FileMode: got %o, want 600", got)
+	}
+}
+
+// TestGetAttrOnUnlinkedOpenFile pins fstat on an unlinked descriptor: the size
+// stays visible and nlink drops to zero.
+func TestGetAttrOnUnlinkedOpenFile(t *testing.T) {
+	wfs, inode, fh := newUnlinkedOpenFile(t)
+	fh.GetEntry().GetEntry().Attributes.FileSize = 128
+
+	in := &fuse.GetAttrIn{}
+	in.NodeId = inode
+	var out fuse.AttrOut
+	if status := wfs.GetAttr(nil, in, &out); status != fuse.OK {
+		t.Fatalf("GetAttr on unlinked open file: got %v, want OK", status)
+	}
+	if out.Attr.Size != 128 {
+		t.Fatalf("GetAttr size: got %d, want 128", out.Attr.Size)
+	}
+	if out.Attr.Nlink != 0 {
+		t.Fatalf("GetAttr nlink: got %d, want 0", out.Attr.Nlink)
+	}
+}
+
+// TestXAttrOnUnlinkedOpenFile covers fsetxattr/fgetxattr/fremovexattr on a
+// descriptor whose file has been unlinked.
+func TestXAttrOnUnlinkedOpenFile(t *testing.T) {
+	wfs, inode, _ := newUnlinkedOpenFile(t)
+
+	setIn := &fuse.SetXAttrIn{}
+	setIn.NodeId = inode
+	if status := wfs.SetXAttr(nil, setIn, "user.k", []byte("v")); status != fuse.OK {
+		t.Fatalf("SetXAttr on unlinked open file: 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 unlinked open file: 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 unlinked open file: got %v, want OK", status)
+	}
+}