Use inode number and size for more robust entries change tracking

Author: kovidgoyal

tools/disk_cache/api.go

@@ -25,14 +25,14 @@ type DiskCache struct {
 	Path    string
 	MaxSize int64
 
-	lock_file        *os.File
-	lock_mutex       sync.Mutex
-	entries          Metadata
-	entry_map        map[string]*Entry
-	entries_mod_time time.Time
-	entries_dirty    bool
-	get_dir          string
-	read_count       int
+	lock_file               *os.File
+	lock_mutex              sync.Mutex
+	entries                 Metadata
+	entry_map               map[string]*Entry
+	entries_last_read_state *file_state
+	entries_dirty           bool
+	get_dir                 string
+	read_count              int
 }
 
 func NewDiskCache(path string, max_size int64) (dc *DiskCache, err error) {

tools/disk_cache/implementation.go

@@ -1,6 +1,7 @@
 package disk_cache
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
@@ -12,11 +13,51 @@ import (
 	"slices"
 	"time"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/kovidgoyal/kitty/tools/utils"
 )
 
 var _ = fmt.Print
 
+type file_state struct {
+	Size    int64
+	ModTime time.Time
+	Inode   uint64
+}
+
+func (s *file_state) equal(o *file_state) bool {
+	return o != nil && s.Size == o.Size && s.ModTime.Equal(o.ModTime) && s.Inode == o.Inode
+}
+
+func get_file_state(fi fs.FileInfo) *file_state {
+	// The Sys() method returns the underlying data source (can be nil).
+	// For Unix-like systems, it's a *syscall.Stat_t.
+	stat, ok := fi.Sys().(*unix.Stat_t)
+	if !ok {
+		// For non-Unix systems, you might not have an inode.
+		// In that case, you can fall back to using only size and mod time.
+		return &file_state{
+			Size:    fi.Size(),
+			ModTime: fi.ModTime(),
+			Inode:   0, // Inode not available
+		}
+	}
+	return &file_state{
+		Size:    fi.Size(),
+		ModTime: fi.ModTime(),
+		Inode:   stat.Ino,
+	}
+}
+
+func get_file_state_from_path(path string) (*file_state, error) {
+	if s, err := os.Stat(path); err != nil {
+		return nil, err
+	} else {
+		return get_file_state(s), nil
+	}
+}
+
 func new_disk_cache(path string, max_size int64) (dc *DiskCache, err error) {
 	if path, err = filepath.Abs(path); err != nil {
 		return
@@ -92,16 +133,20 @@ func (dc *DiskCache) write_entries_if_dirty() (err error) {
 	if !dc.entries_dirty {
 		return
 	}
+	path := dc.entries_path()
 	defer func() {
 		if err == nil {
 			dc.entries_dirty = false
-			dc.entries_mod_time = time.Now()
+			if s, serr := get_file_state_from_path(path); serr == nil {
+				dc.entries_last_read_state = s
+			}
 		}
 	}()
 	if d, err := json.Marshal(dc.entries); err != nil {
 		return err
 	} else {
-		return os.WriteFile(dc.entries_path(), d, 0o600)
+		// use an atomic write so that the inode number changes
+		return utils.AtomicWriteFile(path, bytes.NewReader(d), 0o600)
 	}
 }
 
@@ -156,14 +201,15 @@ func (dc *DiskCache) rebuild_entries() error {
 }
 
 func (dc *DiskCache) ensure_entries() error {
-	needed := dc.entry_map == nil
+	needed := dc.entry_map == nil || dc.entries_last_read_state == nil
 	path := dc.entries_path()
-	var stat_result fs.FileInfo
+	var fstate *file_state
 	if !needed {
-		if s, err := os.Stat(path); err == nil && s.ModTime().After(dc.entries_mod_time) {
-			needed = true
-			stat_result = s
-			dc.entries_mod_time = s.ModTime()
+		if s, err := get_file_state_from_path(path); err == nil {
+			fstate = s
+			if !s.equal(dc.entries_last_read_state) {
+				needed = true
+			}
 		}
 	}
 	if needed {
@@ -181,11 +227,12 @@ func (dc *DiskCache) ensure_entries() error {
 				// corrupted data
 				dc.rebuild_entries()
 			} else {
-				if stat_result == nil {
-					if s, err := os.Stat(path); err == nil {
-						dc.entries_mod_time = s.ModTime()
+				if fstate == nil {
+					if s, err := get_file_state_from_path(path); err == nil {
+						fstate = s
 					}
 				}
+				dc.entries_last_read_state = fstate
 			}
 			dc.entry_map = make(map[string]*Entry)
 			for _, e := range dc.entries.SortedEntries {

tools/disk_cache/implementation_test.go

@@ -77,6 +77,7 @@ func TestDiskCache(t *testing.T) {
 	arc(dc, 1) // because dc2.Get() will have updated the file
 	arc(dc2, 1)
 	ak("k1")
+	arc(dc2, 2) // because dc.Add() will have updated the file
 	dc2.Add("k2", map[string][]byte{"1": []byte("123456789")})
 	arc(dc, 1)
 	arc(dc2, 2)