From 06ccd0e9fc206ca1cf5196a7eea19525e0767541 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 20 Apr 2026 17:54:54 -0700 Subject: [PATCH] fix(mount): flush dirty handles on Release when kernel skipped Flush (#9165) * fix(mount): flush dirty handles on Release when kernel skipped Flush The FUSE protocol allows the kernel to send Release without a preceding Flush; file handles that reach Release with dirtyMetadata=true (notably deferred creates that never saw any write) would then have their pending filer CreateEntry dropped on the floor, leaving the mount and filer out of sync. Detect dirty handles in Release and call doFlush before tearing the handle down. Skip the fallback when an async flush is already pending so we don't double-submit. Flock-unlock Releases stay on the synchronous path so close()-time serialization is preserved. Adds TestReleaseFlushesDirtyCreateIfFlushWasSkipped covering the create-without-flush path. * address review: drop racy dirty-flag peek, let doFlush self-gate fh.dirtyMetadata / fh.asyncFlushPending are written from the periodic metadata flusher and async flush worker under fhLockTable, so the unsynchronized read in Release was a data race per the reviewer. Just call doFlush unconditionally on every Release; it already fast- paths the clean case (dirtyPages.FlushData early-returns when hasWrites is false, and the dirty-metadata branch short-circuits), so the extra call after a normal Flush is cheap while the no-Flush-before-Release path still recovers a deferred create. --- weed/mount/weedfs_file_io.go | 16 ++++++++- weed/mount/weedfs_file_mkrm_test.go | 55 ++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/weed/mount/weedfs_file_io.go b/weed/mount/weedfs_file_io.go index 572a3f05a..c4b615676 100644 --- a/weed/mount/weedfs_file_io.go +++ b/weed/mount/weedfs_file_io.go @@ -1,6 +1,9 @@ package mount -import "github.com/seaweedfs/go-fuse/v2/fuse" +import ( + "github.com/seaweedfs/go-fuse/v2/fuse" + "github.com/seaweedfs/seaweedfs/weed/glog" +) /** * Open a file @@ -133,6 +136,17 @@ func (wfs *WFS) invalidateOpenMtimeCache(inode uint64) { } func (wfs *WFS) Release(cancel <-chan struct{}, in *fuse.ReleaseIn) { + // 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. + // doFlush itself inspects dirtyMetadata / asyncFlushPending and fast-paths + // the clean case, so the duplicate call after a normal Flush is cheap. + if fh := wfs.GetHandle(FileHandleId(in.Fh)); fh != nil { + allowAsync := in.ReleaseFlags&fuse.FUSE_RELEASE_FLOCK_UNLOCK == 0 + if status := wfs.doFlush(fh, in.Uid, in.Gid, allowAsync); status != fuse.OK { + glog.Warningf("release fh %d inode %d: fallback flush failed: %v", in.Fh, in.NodeId, status) + } + } if in.ReleaseFlags&fuse.FUSE_RELEASE_FLOCK_UNLOCK != 0 { wfs.posixLocks.ReleaseFlockOwner(in.NodeId, in.LockOwner) } diff --git a/weed/mount/weedfs_file_mkrm_test.go b/weed/mount/weedfs_file_mkrm_test.go index f7d7600c2..e384eb101 100644 --- a/weed/mount/weedfs_file_mkrm_test.go +++ b/weed/mount/weedfs_file_mkrm_test.go @@ -110,10 +110,10 @@ func newCreateTestWFS(t *testing.T) (*WFS, *createEntryTestServer) { } wfs := &WFS{ - option: option, - signature: 1, - inodeToPath: NewInodeToPath(root, 0), - fhMap: NewFileHandleToInode(), + option: option, + signature: 1, + inodeToPath: NewInodeToPath(root, 0), + fhMap: NewFileHandleToInode(), fhLockTable: util.NewLockTable[FileHandleId](), hardLinkLockTable: util.NewLockTable[string](), } @@ -204,6 +204,53 @@ func TestCreateCreatesAndOpensFile(t *testing.T) { } } +func TestReleaseFlushesDirtyCreateIfFlushWasSkipped(t *testing.T) { + wfs, testServer := newCreateTestWFS(t) + + out := &fuse.CreateOut{} + status := wfs.Create(make(chan struct{}), &fuse.CreateIn{ + InHeader: fuse.InHeader{ + NodeId: 1, + Caller: fuse.Caller{ + Owner: fuse.Owner{ + Uid: 123, + Gid: 456, + }, + }, + }, + Flags: syscall.O_WRONLY | syscall.O_CREAT, + Mode: 0o640, + }, "release_flush.txt", out) + if status != fuse.OK { + t.Fatalf("Create status = %v, want OK", status) + } + + wfs.Release(make(chan struct{}), &fuse.ReleaseIn{ + InHeader: fuse.InHeader{ + NodeId: out.NodeId, + Caller: fuse.Caller{Owner: fuse.Owner{Uid: 123, Gid: 456}}, + }, + Fh: out.Fh, + }) + + snapshot := testServer.snapshot() + if snapshot.directory != "/" { + t.Fatalf("CreateEntry directory = %q, want %q", snapshot.directory, "/") + } + if snapshot.name != "release_flush.txt" { + t.Fatalf("CreateEntry name = %q, want %q", snapshot.name, "release_flush.txt") + } + if snapshot.uid != 123 || snapshot.gid != 456 { + t.Fatalf("CreateEntry uid/gid = %d/%d, want 123/456", snapshot.uid, snapshot.gid) + } + if snapshot.mode != 0o640 { + t.Fatalf("CreateEntry mode = %o, want %o", snapshot.mode, 0o640) + } + if fh := wfs.GetHandle(FileHandleId(out.Fh)); fh != nil { + t.Fatal("Release should remove the file handle after fallback flush") + } +} + func TestTruncateEntryClearsDirtyPagesForOpenHandle(t *testing.T) { wfs, _ := newCreateTestWFS(t)