Promote develop: vfs/cache promoteToFast race fix #24

Merged
Ghost merged 2 commits from develop into main 2026-09-01 15:36:49 -05:00
6 changed files with 94 additions and 8 deletions
+4 -2
View File
@@ -186,7 +186,7 @@ func (tc *TieredCache) Capacity() int64 {
func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
defer func() { _ = reader.Close() }() // best-effort close; error secondary to promotion attempt (async best-effort path)
// Get file info from slow tier to determine size
// Size for the space/ReadAll guards comes from a Stat snapshot, not the live in-map FileInfo.
var size int64
if slow := tc.slow.Load(); slow != nil {
if vfs, ok := slow.(vfs.VFS); ok {
@@ -210,7 +210,7 @@ func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
}
// Guard promotion ReadAll using already-fetched size (in addition to space check above)
if size > 0 && size > (1<<30) { // conservative 1GB hard limit on promotion reads (aligns with typical max_object_size)
if size > (1 << 30) { // conservative 1GB hard limit on promotion reads (aligns with typical max_object_size)
return
}
// Read the entire file content
@@ -218,6 +218,8 @@ func (tc *TieredCache) promoteToFast(key string, reader io.ReadCloser) {
if err != nil {
return // Skip promotion if read fails
}
// Create with the bytes we actually hold so we never reuse a live FileInfo.Size.
size = int64(len(content))
// Create the file in fast tier
if fast := tc.fast.Load(); fast != nil {
+9 -4
View File
@@ -612,7 +612,9 @@ func (d *DiskFS) Delete(key string) error {
return nil
}
// Stat returns file information with lazy discovery
// Stat returns a snapshot of file information with lazy discovery.
// The returned *FileInfo is not the live cache entry; Close may update Size
// on the in-map object under d.mu.
func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
if key == "" {
return nil, vfserror.ErrInvalidKey
@@ -627,9 +629,10 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
keyMu.RLock()
d.mu.RLock()
if fi, ok := d.info[key]; ok {
snap := fi.Clone()
d.mu.RUnlock()
keyMu.RUnlock()
return fi, nil
return snap, nil
}
d.mu.RUnlock()
keyMu.RUnlock()
@@ -649,8 +652,9 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
// Double-check after acquiring write lock
d.mu.Lock()
if fi, ok := d.info[key]; ok {
snap := fi.Clone()
d.mu.Unlock()
return fi, nil
return snap, nil
}
// Re-verify the file still exists on disk under the lock before inserting.
@@ -669,9 +673,10 @@ func (d *DiskFS) Stat(key string) (*vfs.FileInfo, error) {
fi.UpdateAccessBatched(d.timeUpdater)
// Note: size not updated on lazy discovery (preserves prior behavior; initial on-disk accounted via bg populate at New time,
// subsequent files come via Create which accounts size).
snap := fi.Clone()
d.mu.Unlock()
return fi, nil
return snap, nil
}
// EvictLRU evicts the least recently used files to free up space
+3 -2
View File
@@ -289,7 +289,8 @@ func (m *MemoryFS) Delete(key string) error {
return nil
}
// Stat returns file information
// Stat returns a snapshot of file information. The returned *FileInfo is not
// the live cache entry; Close may update Size on the in-map object under m.mu.
func (m *MemoryFS) Stat(key string) (*types.FileInfo, error) {
if key == "" {
return nil, vfserror.ErrInvalidKey
@@ -310,7 +311,7 @@ func (m *MemoryFS) Stat(key string) (*types.FileInfo, error) {
defer m.mu.RUnlock()
if fi, ok := m.info[key]; ok {
return fi, nil
return fi.Clone(), nil
}
return nil, vfserror.ErrNotFound
+39
View File
@@ -346,6 +346,45 @@ func TestMemoryFS_ConcurrentCloseAndEvict_RaceFree(t *testing.T) {
_ = m.LRU.Len()
}
func TestMemoryFS_StatReturnsSnapshot(t *testing.T) {
t.Parallel()
m, err := New(1024)
if err != nil {
t.Fatal(err)
}
w, err := m.Create("k", 10)
if err != nil {
t.Fatal(err)
}
if _, err := w.Write([]byte("hello")); err != nil {
t.Fatal(err)
}
if err := w.Close(); err != nil {
t.Fatal(err)
}
fi, err := m.Stat("k")
if err != nil {
t.Fatal(err)
}
if fi.Size != 5 {
t.Fatalf("size %d want 5", fi.Size)
}
fi.Size = 999
fi.AccessCount = 0
fi2, err := m.Stat("k")
if err != nil {
t.Fatal(err)
}
if fi2.Size != 5 {
t.Errorf("Stat returned live FileInfo; store size became %d", fi2.Size)
}
if fi2.AccessCount == 0 {
t.Error("Stat returned live FileInfo; AccessCount mutation leaked")
}
}
func TestMemoryFS_EvictVariantsAndErrors(t *testing.T) {
t.Parallel()
m, err := New(800)
+11
View File
@@ -27,6 +27,17 @@ func NewFileInfo(key string, size int64) *FileInfo {
}
}
// Clone returns a snapshot copy of fi. Stat returns Clone() so callers can
// read Size and other fields without racing Close/Open mutations of the
// in-map FileInfo.
func (fi *FileInfo) Clone() *FileInfo {
if fi == nil {
return nil
}
cp := *fi
return &cp
}
// NewFileInfoFromOS creates a FileInfo from os.FileInfo
func NewFileInfoFromOS(info os.FileInfo, key string) *FileInfo {
return &FileInfo{
+28
View File
@@ -16,6 +16,34 @@ func TestNewFileInfo(t *testing.T) {
}
}
func TestFileInfoClone(t *testing.T) {
t.Parallel()
fi := NewFileInfo("k", 42)
fi.AccessCount = 7
cp := fi.Clone()
if cp == fi {
t.Fatal("Clone returned the same pointer")
}
if cp.Key != fi.Key || cp.Size != fi.Size || cp.AccessCount != fi.AccessCount {
t.Errorf("Clone mismatch: %+v vs %+v", cp, fi)
}
if !cp.ATime.Equal(fi.ATime) || !cp.CTime.Equal(fi.CTime) {
t.Error("Clone timestamps mismatch")
}
cp.Size = 99
cp.AccessCount = 1
if fi.Size != 42 || fi.AccessCount != 7 {
t.Error("mutating Clone affected original")
}
if NewFileInfo("x", 1).Clone() == nil {
t.Error("Clone of non-nil was nil")
}
var none *FileInfo
if none.Clone() != nil {
t.Error("Clone of nil was non-nil")
}
}
func TestUpdateAccess(t *testing.T) {
t.Parallel()
fi := NewFileInfo("k", 1)