Skip to content

Commit 42ab37b

Browse files
committed
wip: vfs: attempt to fix modtime inconsistencies
Signed-off-by: Anagh Kumar Baranwal <[email protected]>
1 parent 3e50582 commit 42ab37b

File tree

2 files changed

+58
-12
lines changed

2 files changed

+58
-12
lines changed

vfs/file.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -386,15 +386,24 @@ func (f *File) ModTime() (modTime time.Time) {
386386
// Note that we only cache modtime values that we have returned to the OS
387387
// if we haven't returned a value to the OS then we can change it
388388
defer func() {
389-
if f.d.f.Precision() == fs.ModTimeNotSupported && (virtualModTime == nil || !virtualModTime.Equal(modTime)) {
390-
f.virtualModTime = &modTime
391-
fs.Debugf(f._path(), "Set virtual modtime to %v", f.virtualModTime)
389+
if f.d.f.Precision() == fs.ModTimeNotSupported {
390+
f.mu.Lock()
391+
// Re-check virtualModTime under lock to avoid race condition
392+
if f.virtualModTime == nil || !f.virtualModTime.Equal(modTime) {
393+
f.virtualModTime = &modTime
394+
fs.Debugf(f._path(), "Set virtual modtime to %v", f.virtualModTime)
395+
}
396+
f.mu.Unlock()
392397
}
393398
}()
394399

395400
if d.vfs.Opt.NoModTime {
396401
return d.ModTime()
397402
}
403+
// Check pending modtime first (explicit SetModTime calls take precedence)
404+
if !pendingModTime.IsZero() {
405+
return f._roundModTime(pendingModTime)
406+
}
398407
// Read the modtime from a dirty item if it exists
399408
if f.d.vfs.Opt.CacheMode >= vfscommon.CacheModeMinimal {
400409
if item := f.d.vfs.cache.DirtyItem(f._cachePath()); item != nil {
@@ -406,11 +415,8 @@ func (f *File) ModTime() (modTime time.Time) {
406415
}
407416
}
408417
}
409-
if !pendingModTime.IsZero() {
410-
return f._roundModTime(pendingModTime)
411-
}
412418
if virtualModTime != nil && !virtualModTime.IsZero() {
413-
fs.Debugf(f._path(), "Returning virtual modtime %v", f.virtualModTime)
419+
fs.Debugf(f._path(), "Returning virtual modtime %v", *virtualModTime)
414420
return f._roundModTime(*virtualModTime)
415421
}
416422
if o == nil {

vfs/vfscache/item.go

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,8 @@ func (item *Item) Truncate(size int64) (err error) {
367367
//
368368
// Call with mutex held
369369
func (item *Item) _stat() (fi os.FileInfo, err error) {
370-
if item.fd != nil {
371-
return item.fd.Stat()
372-
}
370+
// Always use os.Stat() to avoid stale file descriptor cache
371+
// after os.Chtimes() calls in _setModTime()
373372
osPath := item.c.toOSPath(item.name) // No locking in Cache
374373
return os.Stat(osPath)
375374
}
@@ -624,6 +623,28 @@ func (item *Item) _store(ctx context.Context, storeFn StoreFn) (err error) {
624623
item.mu.Lock()
625624
}
626625

626+
// Ensure remote modtime matches cache metadata
627+
// This handles the implicit modtime case where no explicit SetModTime was called
628+
// For explicit SetModTime, storeFn already updated both item.info.ModTime and remote
629+
if item.o != nil {
630+
intendedModTime := item.info.ModTime
631+
actualModTime := item.o.ModTime(context.Background())
632+
precision := item.o.Fs().Precision()
633+
634+
// Check if modtimes differ beyond precision tolerance
635+
dt := intendedModTime.Sub(actualModTime)
636+
if dt >= precision || dt <= -precision {
637+
fs.Debugf(item.name, "vfs cache: setting remote modtime to %v (was %v)", intendedModTime, actualModTime)
638+
err = item.o.SetModTime(context.Background(), intendedModTime)
639+
if err != nil {
640+
// Don't fail the upload if we can't set modtime
641+
if !errors.Is(err, fs.ErrorCantSetModTime) && !errors.Is(err, fs.ErrorCantSetModTimeWithoutDelete) {
642+
fs.Errorf(item.name, "vfs cache: failed to set remote modtime: %v", err)
643+
}
644+
}
645+
}
646+
}
647+
627648
// Show item is clean and is eligible for cache removal
628649
item.info.Dirty = false
629650
err = item._save()
@@ -650,13 +671,17 @@ func (item *Item) Close(storeFn StoreFn) (err error) {
650671
var (
651672
downloaders *downloaders.Downloaders
652673
syncWriteBack = item.c.opt.WriteBack <= 0
674+
wasDirty bool // capture dirty state for modtime decision
653675
)
654676
item.mu.Lock()
655677
defer item.mu.Unlock()
656678

657679
item.info.ATime = time.Now()
658680
item.opens--
659681

682+
// Capture dirty state early for modtime decision later
683+
wasDirty = item.info.Dirty
684+
660685
if item.opens < 0 {
661686
return os.ErrClosed
662687
} else if item.opens > 0 {
@@ -719,7 +744,7 @@ func (item *Item) Close(storeFn StoreFn) (err error) {
719744
// if the item hasn't been changed but has been completed then
720745
// set the modtime from the object otherwise set it from the info
721746
if item._exists() {
722-
if !item.info.Dirty && item.o != nil {
747+
if !wasDirty && item.o != nil {
723748
item._setModTime(item.o.ModTime(context.Background()))
724749
} else {
725750
item._setModTime(item.info.ModTime)
@@ -1220,21 +1245,36 @@ func (item *Item) _setModTime(modTime time.Time) {
12201245
func (item *Item) setModTime(modTime time.Time) {
12211246
// defer log.Trace(item.name, "modTime=%v", modTime)("")
12221247
item.mu.Lock()
1248+
defer item.mu.Unlock()
1249+
12231250
item._updateFingerprint()
12241251
item._setModTime(modTime)
1252+
1253+
// Only update in-memory metadata if disk save succeeds
1254+
// to maintain consistency between in-memory and disk state
1255+
oldModTime := item.info.ModTime
12251256
item.info.ModTime = modTime
12261257
err := item._save()
12271258
if err != nil {
1259+
// Rollback in-memory state on save failure
1260+
item.info.ModTime = oldModTime
12281261
fs.Errorf(item.name, "vfs cache: setModTime: failed to save item info: %v", err)
12291262
}
1230-
item.mu.Unlock()
12311263
}
12321264

12331265
// GetModTime of the cache file
12341266
func (item *Item) GetModTime() (modTime time.Time, err error) {
12351267
// defer log.Trace(item.name, "modTime=%v", modTime)("")
12361268
item.mu.Lock()
12371269
defer item.mu.Unlock()
1270+
1271+
// For dirty files, use the metadata modtime as the authoritative source
1272+
// to avoid race conditions with filesystem modtime updates
1273+
if item.info.Dirty {
1274+
return item.info.ModTime, nil
1275+
}
1276+
1277+
// For clean files, use the filesystem modtime
12381278
fi, err := item._stat()
12391279
if err == nil {
12401280
modTime = fi.ModTime()

0 commit comments

Comments
 (0)