diff --git a/test/winfsp/semantics_test.go b/test/winfsp/semantics_test.go index 04abbdf95..12bf3fb26 100644 --- a/test/winfsp/semantics_test.go +++ b/test/winfsp/semantics_test.go @@ -180,11 +180,31 @@ func TestRenameOverExisting(t *testing.T) { // they indict different layers; a second look says whether it persists. time.Sleep(200 * time.Millisecond) fi2, err2 := os.Stat(src) - t.Fatalf("stat of the renamed-away source did not return not-exist: stat=%v err=%v; 200ms later stat=%v err=%v", - describeFileInfo(fi), err, describeFileInfo(fi2), err2) + // A listing reads no per-path cache and the mount's own forgets + // within a second, so a name that survives both is back on the filer. + listed := dirNames(t, dir) + time.Sleep(2 * time.Second) + _, errLater := os.Stat(src) + t.Fatalf("stat of the renamed-away source did not return not-exist: stat=%v err=%v; 200ms later stat=%v err=%v; past the path cache err=%v; %s lists %v", + describeFileInfo(fi), err, describeFileInfo(fi2), err2, errLater, dir, listed) } } +// dirNames lists a directory for a failure message, reporting the error in +// place of the names rather than failing a test that is already failing. +func dirNames(t *testing.T, dir string) []string { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + return []string{"readdir: " + err.Error()} + } + names := make([]string, 0, len(entries)) + for _, entry := range entries { + names = append(names, entry.Name()) + } + return names +} + func TestRenameAcrossDirectories(t *testing.T) { dir := testRoot(t) from := filepath.Join(dir, "from") diff --git a/weed/mount/filehandle.go b/weed/mount/filehandle.go index 98f7b274a..a2c5560d4 100644 --- a/weed/mount/filehandle.go +++ b/weed/mount/filehandle.go @@ -40,7 +40,11 @@ type FileHandle struct { savedName string // last known file name if inode-to-path state is forgotten isDeleted bool - isRenamed bool // set by Rename before waiting for async flush; skips old-path metadata flush + // deleteEpoch counts the times isDeleted was raised, all of them under the + // handle's flush lock. A caller that raised it and then found it had + // nothing to delete after all can tell its own mark from a later one. + deleteEpoch uint64 + isRenamed bool // set by Rename before waiting for async flush; skips old-path metadata flush // entryVersionTsNs is the filer log position the handle's entry reflects. // State at or below it must not replace the entry — that rolls it back. diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index 7a6b14168..45dc75d66 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -938,6 +938,7 @@ func (wfs *WFS) invalidateOpenFileHandle(invalidation meta_cache.EntryInvalidati // dirty pages stay, so the open fd still reads its buffered writes. if invalidation.Deleted { fh.isDeleted = true + fh.deleteEpoch++ } if !fh.dirtyMetadata { fh.dirtyPages.Destroy() diff --git a/weed/mount/weedfs_file_mkrm.go b/weed/mount/weedfs_file_mkrm.go index 316642bfc..d00f479d4 100644 --- a/weed/mount/weedfs_file_mkrm.go +++ b/weed/mount/weedfs_file_mkrm.go @@ -436,17 +436,39 @@ func (wfs *WFS) createRegularFile(dirFullPath util.FullPath, name string, mode u } // markHandleDeleted flags the inode's open handle so its flushes stop writing -// the entry back. Taken and released under the handle's flush lock: a flush -// already holding it finishes before the caller's delete runs, and any later -// flush sees the flag; setting the flag bare raced the flush's own check and -// resurrected the entry right after the delete. -func (wfs *WFS) markHandleDeleted(inode uint64) { +// the entry back, and reports which mark this was. Taken and released under +// the handle's flush lock: a flush already holding it finishes before the +// caller's delete runs, and any later flush sees the flag; setting the flag +// bare raced the flush's own check and resurrected the entry right after the +// delete. +func (wfs *WFS) markHandleDeleted(inode uint64) uint64 { + fh, found := wfs.fhMap.FindFileHandle(inode) + if !found { + return 0 + } + fhActiveLock := wfs.fhLockTable.AcquireLock("markHandleDeleted", fh.fh, util.ExclusiveLock) + fh.isDeleted = true + fh.deleteEpoch++ + epoch := fh.deleteEpoch + wfs.fhLockTable.ReleaseLock(fh.fh, fhActiveLock) + return epoch +} + +// clearHandleDeleted lifts a mark whose delete did not go through. Only the +// mark epoch names: an unlink that raised the flag after it owns the entry's +// fate and keeps it. +func (wfs *WFS) clearHandleDeleted(inode uint64, epoch uint64) { + if epoch == 0 { + return + } fh, found := wfs.fhMap.FindFileHandle(inode) if !found { return } - fhActiveLock := wfs.fhLockTable.AcquireLock("Unlink", fh.fh, util.ExclusiveLock) - fh.isDeleted = true + fhActiveLock := wfs.fhLockTable.AcquireLock("clearHandleDeleted", fh.fh, util.ExclusiveLock) + if fh.deleteEpoch == epoch { + fh.isDeleted = false + } wfs.fhLockTable.ReleaseLock(fh.fh, fhActiveLock) } diff --git a/weed/mount/weedfs_rename.go b/weed/mount/weedfs_rename.go index 995d34417..e4fd86682 100644 --- a/weed/mount/weedfs_rename.go +++ b/weed/mount/weedfs_rename.go @@ -214,17 +214,22 @@ func (wfs *WFS) Rename(cancel <-chan struct{}, in *fuse.RenameIn, oldName string } } - // POSIX: enforce sticky bit on the destination directory when replacing an existing entry. + var newEntry *filer_pb.Entry if in.Flags != RenameNoReplace { - if newEntry, _, newStatus := wfs.maybeLoadEntry(newPath); newStatus == fuse.OK && newEntry != nil { - if newDirEntry, _, dirCode := wfs.maybeLoadEntry(newDir); dirCode == fuse.OK && newDirEntry != nil && newDirEntry.Attributes != nil { - targetUid := uint32(0) - if newEntry.Attributes != nil { - targetUid = newEntry.Attributes.Uid - } - if code := checkStickyBit(newDirEntry.Attributes.FileMode, newDirEntry.Attributes.Uid, targetUid, in.Uid); code != fuse.OK { - return code - } + if loaded, _, newStatus := wfs.maybeLoadEntry(newPath); newStatus == fuse.OK { + newEntry = loaded + } + } + + // POSIX: enforce sticky bit on the destination directory when replacing an existing entry. + if newEntry != nil { + if newDirEntry, _, dirCode := wfs.maybeLoadEntry(newDir); dirCode == fuse.OK && newDirEntry != nil && newDirEntry.Attributes != nil { + targetUid := uint32(0) + if newEntry.Attributes != nil { + targetUid = newEntry.Attributes.Uid + } + if code := checkStickyBit(newDirEntry.Attributes.FileMode, newDirEntry.Attributes.Uid, targetUid, in.Uid); code != fuse.OK { + return code } } } @@ -240,10 +245,11 @@ func (wfs *WFS) Rename(cancel <-chan struct{}, in *fuse.RenameIn, oldName string // 1. deferFilerCreate=true — file handle still open, dirtyMetadata set. // 2. writebackCache — close() triggered async flush, handle released. // The filer rename will fail with ENOENT unless we flush/wait first. - if inode, found := wfs.inodeToPath.GetInode(oldPath); found { + sourceInode, sourceMapped := wfs.inodeToPath.GetInode(oldPath) + if sourceMapped { // Case 1: handle still open with deferred metadata — flush synchronously // BEFORE any async flush interference. - if fh, ok := wfs.fhMap.FindFileHandle(inode); ok && fh.dirtyMetadata { + if fh, ok := wfs.fhMap.FindFileHandle(sourceInode); ok && fh.dirtyMetadata { glog.V(4).Infof("dir Rename %s: flushing deferred metadata before rename", oldPath) // Prerequisite for the rename, so it must complete: non-cancellable context. if flushStatus := wfs.doFlush(context.Background(), fh, oldEntry.Attributes.Uid, oldEntry.Attributes.Gid, false); flushStatus != fuse.OK { @@ -255,14 +261,43 @@ func (wfs *WFS) Rename(cancel <-chan struct{}, in *fuse.RenameIn, oldName string // Mark ALL handles for this inode as renamed so the async flush // skips old-path metadata creation (which would re-insert the // renamed entry into the meta cache after rename events clean it up). - wfs.fhMap.MarkInodeRenamed(inode) - wfs.waitForPendingAsyncFlush(inode) + wfs.fhMap.MarkInodeRenamed(sourceInode) + wfs.waitForPendingAsyncFlush(sourceInode) } else if oldEntry != nil && oldEntry.Attributes != nil && oldEntry.Attributes.Inode != 0 { // GetInode failed (Forget already removed the mapping), but the // entry's stored inode can still identify pending async flushes. - inode = oldEntry.Attributes.Inode - wfs.fhMap.MarkInodeRenamed(inode) - wfs.waitForPendingAsyncFlush(inode) + sourceInode = oldEntry.Attributes.Inode + wfs.fhMap.MarkInodeRenamed(sourceInode) + wfs.waitForPendingAsyncFlush(sourceInode) + } + + // Replacing the destination deletes what it held, so a handle still open + // on that entry has to stop writing the name back, exactly as an unlink + // makes it. Windows is where this bites: the close carrying the flush runs + // after the application's CloseHandle returned, so the flush can land on + // top of what the rename just put there. + if in.Flags == RenameEmptyFlag { + targetInode, targetMapped := wfs.inodeToPath.GetInode(newPath) + if !targetMapped && newEntry != nil && newEntry.Attributes != nil { + // Forget dropped the mapping, but the entry's stored inode still + // names the handle, the way the source side falls back. + targetInode = newEntry.Attributes.Inode + } + if targetInode != 0 && targetInode != sourceInode { + // Drained before the mark, not after: a flush already queued + // belongs to the destination as it stands, and one that runs + // marked is skipped and takes its handle with it, past the reach + // of the restore below. + wfs.waitForPendingAsyncFlush(targetInode) + epoch := wfs.markHandleDeleted(targetInode) + // A rename that does not happen leaves the destination where it + // was, so its handle has to go back to writing that name. + defer func() { + if code != fuse.OK { + wfs.clearHandleDeleted(targetInode, epoch) + } + }() + } } // Acquire DLM locks on both old and new paths to prevent another mount