From f73f09bb7a03297ebcbae6a4cf8efc7897664781 Mon Sep 17 00:00:00 2001 From: John Xu Date: Mon, 6 Oct 2025 13:52:27 +0800 Subject: [PATCH 1/3] Fix nil pointer dereference in snapshotFS.GetFile for non-existent files --- internal/project/snapshot_test.go | 24 ++++++++++++++++++++++++ internal/project/snapshotfs.go | 8 +++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/internal/project/snapshot_test.go b/internal/project/snapshot_test.go index a075039b33..87605530b2 100644 --- a/internal/project/snapshot_test.go +++ b/internal/project/snapshot_test.go @@ -101,4 +101,28 @@ func TestSnapshot(t *testing.T) { assert.Check(t, snapshotAfter.fs.diskFiles["/home/projects/ts/p1/a.ts"] == nil) assert.Check(t, snapshotAfter.fs.diskFiles["/home/projects/ts/p2/b.ts"] != nil) }) + + t.Run("GetFile returns nil for non-existent files", func(t *testing.T) { + t.Parallel() + files := map[string]any{ + "/home/projects/TS/p1/tsconfig.json": "{}", + "/home/projects/TS/p1/index.ts": "console.log('Hello, world!');", + } + session := setup(files) + session.DidOpenFile(context.Background(), "file:///home/projects/TS/p1/index.ts", 1, files["/home/projects/TS/p1/index.ts"].(string), lsproto.LanguageKindTypeScript) + snapshot, release := session.Snapshot() + defer release() + + // Test that GetFile returns nil for non-existent file (first call) + handle := snapshot.GetFile("/home/projects/TS/p1/nonexistent.ts") + assert.Check(t, handle == nil, "GetFile should return nil for non-existent file on first call") + + // Test that GetFile returns nil for non-existent file (second call - triggers the cached path) + handle2 := snapshot.GetFile("/home/projects/TS/p1/nonexistent.ts") + assert.Check(t, handle2 == nil, "GetFile should return nil for non-existent file on second call") + + // Test that ReadFile returns false for non-existent file + _, ok := snapshot.ReadFile("/home/projects/TS/p1/nonexistent.ts") + assert.Check(t, !ok, "ReadFile should return false for non-existent file") + }) } diff --git a/internal/project/snapshotfs.go b/internal/project/snapshotfs.go index 1b45acf0a2..79fd5cef85 100644 --- a/internal/project/snapshotfs.go +++ b/internal/project/snapshotfs.go @@ -49,10 +49,12 @@ func (s *snapshotFS) GetFile(fileName string) FileHandle { } return nil })) - if entry, ok := s.readFiles.LoadOrStore(s.toPath(fileName), newEntry); ok { - return entry() + entry, _ := s.readFiles.LoadOrStore(s.toPath(fileName), newEntry) + file := entry() + if file == nil { + return nil } - return nil + return file } type snapshotFSBuilder struct { From 7d116ce13dc2e87f48ee915ad9157757719ce6cb Mon Sep 17 00:00:00 2001 From: John Xu Date: Mon, 6 Oct 2025 13:55:52 +0800 Subject: [PATCH 2/3] Simplify GetFile nil check for non-existent files in snapshot tests --- internal/project/snapshot_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/project/snapshot_test.go b/internal/project/snapshot_test.go index 87605530b2..ded5e16b5e 100644 --- a/internal/project/snapshot_test.go +++ b/internal/project/snapshot_test.go @@ -113,13 +113,8 @@ func TestSnapshot(t *testing.T) { snapshot, release := session.Snapshot() defer release() - // Test that GetFile returns nil for non-existent file (first call) handle := snapshot.GetFile("/home/projects/TS/p1/nonexistent.ts") - assert.Check(t, handle == nil, "GetFile should return nil for non-existent file on first call") - - // Test that GetFile returns nil for non-existent file (second call - triggers the cached path) - handle2 := snapshot.GetFile("/home/projects/TS/p1/nonexistent.ts") - assert.Check(t, handle2 == nil, "GetFile should return nil for non-existent file on second call") + assert.Check(t, handle == nil, "GetFile should return nil for non-existent file") // Test that ReadFile returns false for non-existent file _, ok := snapshot.ReadFile("/home/projects/TS/p1/nonexistent.ts") From ec09ad204eb030cbd645d867965a096a5240d6c9 Mon Sep 17 00:00:00 2001 From: John Xu Date: Mon, 6 Oct 2025 14:05:00 +0800 Subject: [PATCH 3/3] Refactor memoizedDiskFile type to return FileHandle instead of *diskFile in snapshotFS --- internal/project/snapshotfs.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/project/snapshotfs.go b/internal/project/snapshotfs.go index 79fd5cef85..441299e318 100644 --- a/internal/project/snapshotfs.go +++ b/internal/project/snapshotfs.go @@ -30,7 +30,7 @@ type snapshotFS struct { readFiles collections.SyncMap[tspath.Path, memoizedDiskFile] } -type memoizedDiskFile func() *diskFile +type memoizedDiskFile func() FileHandle func (s *snapshotFS) FS() vfs.FS { return s.fs @@ -43,18 +43,14 @@ func (s *snapshotFS) GetFile(fileName string) FileHandle { if file, ok := s.diskFiles[s.toPath(fileName)]; ok { return file } - newEntry := memoizedDiskFile(sync.OnceValue(func() *diskFile { + newEntry := memoizedDiskFile(sync.OnceValue(func() FileHandle { if contents, ok := s.fs.ReadFile(fileName); ok { return newDiskFile(fileName, contents) } return nil })) entry, _ := s.readFiles.LoadOrStore(s.toPath(fileName), newEntry) - file := entry() - if file == nil { - return nil - } - return file + return entry() } type snapshotFSBuilder struct {