fix(vfs): resolve modtime race conditions in cache layer

Fix multiple interrelated race conditions in VFS modtime handling that
caused incorrect timestamps to be returned after file modifications,
particularly when using the VFS cache.

Issues fixed:

1. Pending vs dirty item priority: pendingModTime (from explicit
   SetModTime calls) now takes precedence over dirty item modtime.
   Previously, dirty item modtime could override an explicit setting.

2. Virtual modtime race: The deferred function setting virtualModTime
   now acquires the mutex and re-checks the condition, preventing
   races between concurrent ModTime() calls.

3. Stale fd stat: Cache item _stat() now always uses os.Stat() on the
   path instead of fd.Stat(). File descriptor stat can return stale
   cached values after os.Chtimes() updates the file's modtime.

4. Remote modtime sync: After uploading a dirty cache item, explicitly
   sync the remote object's modtime to match item.info.ModTime. This
   handles the case where no explicit SetModTime was called but the
   cache has a modtime that should be preserved.

5. Dirty file GetModTime: For dirty files, return the metadata modtime
   directly rather than stat'ing the filesystem, avoiding races with
   concurrent modtime updates.

6. Save failure rollback: setModTime() now rolls back the in-memory
   modtime if the disk metadata save fails, keeping memory and disk
   state consistent.

Signed-off-by: Anagh Kumar Baranwal <[email protected]>

Author: darthShadow

vfs/file.go

@@ -386,15 +386,24 @@ func (f *File) ModTime() (modTime time.Time) {
 	// Note that we only cache modtime values that we have returned to the OS
 	// if we haven't returned a value to the OS then we can change it
 	defer func() {
-		if f.d.f.Precision() == fs.ModTimeNotSupported && (virtualModTime == nil || !virtualModTime.Equal(modTime)) {
-			f.virtualModTime = &modTime
-			fs.Debugf(f._path(), "Set virtual modtime to %v", f.virtualModTime)
+		if f.d.f.Precision() == fs.ModTimeNotSupported {
+			f.mu.Lock()
+			// Re-check virtualModTime under lock to avoid race condition
+			if f.virtualModTime == nil || !f.virtualModTime.Equal(modTime) {
+				f.virtualModTime = &modTime
+				fs.Debugf(f._path(), "Set virtual modtime to %v", f.virtualModTime)
+			}
+			f.mu.Unlock()
 		}
 	}()
 
 	if d.vfs.Opt.NoModTime {
 		return d.ModTime()
 	}
+	// Check pending modtime first (explicit SetModTime calls take precedence)
+	if !pendingModTime.IsZero() {
+		return f._roundModTime(pendingModTime)
+	}
 	// Read the modtime from a dirty item if it exists
 	if f.d.vfs.Opt.CacheMode >= vfscommon.CacheModeMinimal {
 		if item := f.d.vfs.cache.DirtyItem(f._cachePath()); item != nil {
@@ -406,11 +415,8 @@ func (f *File) ModTime() (modTime time.Time) {
 			}
 		}
 	}
-	if !pendingModTime.IsZero() {
-		return f._roundModTime(pendingModTime)
-	}
 	if virtualModTime != nil && !virtualModTime.IsZero() {
-		fs.Debugf(f._path(), "Returning virtual modtime %v", f.virtualModTime)
+		fs.Debugf(f._path(), "Returning virtual modtime %v", *virtualModTime)
 		return f._roundModTime(*virtualModTime)
 	}
 	if o == nil {

vfs/vfscache/item.go

@@ -367,9 +367,8 @@ func (item *Item) Truncate(size int64) (err error) {
 //
 // Call with mutex held
 func (item *Item) _stat() (fi os.FileInfo, err error) {
-	if item.fd != nil {
-		return item.fd.Stat()
-	}
+	// Always use os.Stat() to avoid stale file descriptor cache
+	// after os.Chtimes() calls in _setModTime()
 	osPath := item.c.toOSPath(item.name) // No locking in Cache
 	return os.Stat(osPath)
 }
@@ -624,6 +623,28 @@ func (item *Item) _store(ctx context.Context, storeFn StoreFn) (err error) {
 		item.mu.Lock()
 	}
 
+	// Ensure remote modtime matches cache metadata
+	// This handles the implicit modtime case where no explicit SetModTime was called
+	// For explicit SetModTime, storeFn already updated both item.info.ModTime and remote
+	if item.o != nil {
+		intendedModTime := item.info.ModTime
+		actualModTime := item.o.ModTime(context.Background())
+		precision := item.o.Fs().Precision()
+
+		// Check if modtimes differ beyond precision tolerance
+		dt := intendedModTime.Sub(actualModTime)
+		if dt >= precision || dt <= -precision {
+			fs.Debugf(item.name, "vfs cache: setting remote modtime to %v (was %v)", intendedModTime, actualModTime)
+			err = item.o.SetModTime(context.Background(), intendedModTime)
+			if err != nil {
+				// Don't fail the upload if we can't set modtime
+				if !errors.Is(err, fs.ErrorCantSetModTime) && !errors.Is(err, fs.ErrorCantSetModTimeWithoutDelete) {
+					fs.Errorf(item.name, "vfs cache: failed to set remote modtime: %v", err)
+				}
+			}
+		}
+	}
+
 	// Show item is clean and is eligible for cache removal
 	item.info.Dirty = false
 	err = item._save()
@@ -650,13 +671,17 @@ func (item *Item) Close(storeFn StoreFn) (err error) {
 	var (
 		downloaders   *downloaders.Downloaders
 		syncWriteBack = item.c.opt.WriteBack <= 0
+		wasDirty      bool // capture dirty state for modtime decision
 	)
 	item.mu.Lock()
 	defer item.mu.Unlock()
 
 	item.info.ATime = time.Now()
 	item.opens--
 
+	// Capture dirty state early for modtime decision later
+	wasDirty = item.info.Dirty
+
 	if item.opens < 0 {
 		return os.ErrClosed
 	} else if item.opens > 0 {
@@ -719,7 +744,7 @@ func (item *Item) Close(storeFn StoreFn) (err error) {
 	// if the item hasn't been changed but has been completed then
 	// set the modtime from the object otherwise set it from the info
 	if item._exists() {
-		if !item.info.Dirty && item.o != nil {
+		if !wasDirty && item.o != nil {
 			item._setModTime(item.o.ModTime(context.Background()))
 		} else {
 			item._setModTime(item.info.ModTime)
@@ -1220,21 +1245,36 @@ func (item *Item) _setModTime(modTime time.Time) {
 func (item *Item) setModTime(modTime time.Time) {
 	// defer log.Trace(item.name, "modTime=%v", modTime)("")
 	item.mu.Lock()
+	defer item.mu.Unlock()
+
 	item._updateFingerprint()
 	item._setModTime(modTime)
+
+	// Only update in-memory metadata if disk save succeeds
+	// to maintain consistency between in-memory and disk state
+	oldModTime := item.info.ModTime
 	item.info.ModTime = modTime
 	err := item._save()
 	if err != nil {
+		// Rollback in-memory state on save failure
+		item.info.ModTime = oldModTime
 		fs.Errorf(item.name, "vfs cache: setModTime: failed to save item info: %v", err)
 	}
-	item.mu.Unlock()
 }
 
 // GetModTime of the cache file
 func (item *Item) GetModTime() (modTime time.Time, err error) {
 	// defer log.Trace(item.name, "modTime=%v", modTime)("")
 	item.mu.Lock()
 	defer item.mu.Unlock()
+
+	// For dirty files, use the metadata modtime as the authoritative source
+	// to avoid race conditions with filesystem modtime updates
+	if item.info.Dirty {
+		return item.info.ModTime, nil
+	}
+
+	// For clean files, use the filesystem modtime
 	fi, err := item._stat()
 	if err == nil {
 		modTime = fi.ModTime()