fix: release speculative parse-cache acquire using the acquired file's own key (#4285)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: RyanCavanaugh <6685088+RyanCavanaugh@users.noreply.github.com>

Author: Copilot

internal/compiler/program.go

@@ -281,7 +281,11 @@ func NewProgram(opts ProgramOptions) *Program {
 // In addition to a new program, return a boolean indicating whether the data of the old program was reused.
 // createCheckerPool, if non-nil, overrides the CreateCheckerPool stored in the old program's options,
 // ensuring each caller uses a fresh closure and avoiding data races on captured variables.
-func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHost, createCheckerPool func(*Program) CheckerPool) (*Program, bool) {
+// The returned *ast.SourceFile is the changed file as acquired through newHost; it is nil
+// only if the host cannot locate the file (e.g. it was deleted). Callers that manage
+// host-side parse caches must release this exact pointer when the old program could not be
+// reused, since it was acquired speculatively before that decision was made.
+func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHost, createCheckerPool func(*Program) CheckerPool) (*Program, *ast.SourceFile, bool) {
 	newOpts := p.opts
 	newOpts.Host = newHost
 	if createCheckerPool != nil {
@@ -297,11 +301,11 @@ func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHos
 	_, inRedirectFiles := p.redirectFilesByPath[changedFilePath]
 	_, isRedirectTarget := p.redirectTargetsMap[changedFilePath]
 	if inRedirectFiles || isRedirectTarget {
-		return NewProgram(newOpts), false
+		return NewProgram(newOpts), newFile, false
 	}
 
 	if !canReplaceFileInProgram(oldFile, newFile) {
-		return NewProgram(newOpts), false
+		return NewProgram(newOpts), newFile, false
 	}
 	// TODO: reverify compiler options when config has changed?
 	result := &Program{
@@ -322,7 +326,7 @@ func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHos
 	result.filesByPath = maps.Clone(result.filesByPath)
 	result.filesByPath[newFile.Path()] = newFile
 	updateFileIncludeProcessor(result)
-	return result, true
+	return result, newFile, true
 }
 
 func (p *Program) initCheckerPool() {

internal/project/logging/logger.go

@@ -124,6 +124,12 @@ func NewLogger(output io.Writer) Logger {
 	}
 }
 
+// NewNopLogger returns a no-op Logger that discards all log messages.
+// It is safe to call any method on the returned Logger.
+func NewNopLogger() Logger {
+	return (*logger)(nil)
+}
+
 func formatTime(t time.Time) string {
 	return fmt.Sprintf("[%s]", t.Format("15:04:05.000"))
 }

internal/project/project.go

@@ -366,23 +366,26 @@ func (p *Project) CreateProgram() CreateProgramResult {
 	commandLine := p.getCommandLineWithTypingsFiles()
 
 	if p.dirtyFilePath != "" && p.Program != nil && p.Program.CommandLine() == commandLine {
-		newProgram, programCloned = p.Program.UpdateProgram(p.dirtyFilePath, p.host, createCheckerPool)
+		var dirtyFile *ast.SourceFile
+		newProgram, dirtyFile, programCloned = p.Program.UpdateProgram(p.dirtyFilePath, p.host, createCheckerPool)
 		if programCloned {
 			updateKind = ProgramUpdateKindCloned
 			for _, file := range newProgram.SourceFiles() {
-				if file.Path() != p.dirtyFilePath {
+				// Use pointer identity: dirtyFile is the exact instance UpdateProgram acquired,
+				// and it is the only file whose refcount is already accounted for.
+				if file != dirtyFile {
 					// UpdateProgram acquired the changed file only, so we need to ref everything else
 					p.host.builder.parseCache.Ref(NewParseCacheKey(file.ParseOptions(), file.Hash, file.ScriptKind))
 				}
 			}
 			for _, file := range newProgram.DuplicateSourceFiles() {
 				p.host.builder.parseCache.Ref(NewParseCacheKey(file.ParseOptions, file.Hash, file.ScriptKind))
 			}
-		} else if newFile := newProgram.GetSourceFileByPath(p.dirtyFilePath); newFile != nil {
+		} else if dirtyFile != nil {
 			// UpdateProgram always acquires the dirty file before deciding whether it can
 			// reuse the old program. If it falls back to a full rebuild, release that
 			// speculative acquire so the rebuilt program is the only remaining owner.
-			p.host.builder.parseCache.Deref(NewParseCacheKey(newFile.ParseOptions(), newFile.Hash, newFile.ScriptKind))
+			p.host.builder.parseCache.Deref(NewParseCacheKey(dirtyFile.ParseOptions(), dirtyFile.Hash, dirtyFile.ScriptKind))
 		}
 	} else {
 		var typingsLocation string

internal/project/session.go

@@ -189,12 +189,16 @@ func NewSession(init *SessionInit) *Session {
 	}
 	extendedConfigCache := NewExtendedConfigCache()
 
+	sessionLogger := init.Logger
+	if sessionLogger == nil {
+		sessionLogger = logging.NewNopLogger()
+	}
 	session := &Session{
 		backgroundCtx:       init.BackgroundCtx,
 		options:             init.Options,
 		toPath:              toPath,
 		client:              init.Client,
-		logger:              init.Logger,
+		logger:              sessionLogger,
 		npmExecutor:         init.NpmExecutor,
 		fs:                  overlayFS,
 		parseCache:          parseCache,

internal/project/snapshot_test.go

@@ -171,6 +171,45 @@ func TestSnapshot(t *testing.T) {
 		_, err := session.GetCurrentLanguageServiceWithAutoImports(ctx, otherIndexURI)
 		assert.NilError(t, err)
 	})
+
+	t.Run("fallback rebuild with recomputed parse options is safe for later clone", func(t *testing.T) {
+		t.Parallel()
+
+		testFiles := map[string]any{
+			"/project/node_modules/pkg/index.ts": `export const pkg = 0;`,
+			"/project/src/other.ts":              `export const other = 1;`,
+		}
+		session := setup(testFiles)
+		pkgURI := lsproto.DocumentUri("file:///project/node_modules/pkg/index.ts")
+		otherURI := lsproto.DocumentUri("file:///project/src/other.ts")
+
+		session.DidOpenFile(context.Background(), pkgURI, 1, testFiles["/project/node_modules/pkg/index.ts"].(string), lsproto.LanguageKindTypeScript)
+		session.DidOpenFile(context.Background(), otherURI, 1, testFiles["/project/src/other.ts"].(string), lsproto.LanguageKindTypeScript)
+		_, err := session.GetLanguageService(context.Background(), pkgURI)
+		assert.NilError(t, err)
+
+		err = session.fs.fs.WriteFile("/project/node_modules/pkg/package.json", `{ "type": "module" }`)
+		assert.NilError(t, err)
+		session.DidChangeFile(context.Background(), pkgURI, 2, []lsproto.TextDocumentContentChangePartialOrWholeDocument{
+			{
+				WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{
+					Text: `import "./missing"; export const pkg = 1;`,
+				},
+			},
+		})
+		_, err = session.GetLanguageService(context.Background(), pkgURI)
+		assert.NilError(t, err)
+
+		session.DidChangeFile(context.Background(), otherURI, 2, []lsproto.TextDocumentContentChangePartialOrWholeDocument{
+			{
+				WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{
+					Text: `export const other = 2;`,
+				},
+			},
+		})
+		_, err = session.GetLanguageService(context.Background(), otherURI)
+		assert.NilError(t, err)
+	})
 }
 
 func BenchmarkSnapshotCloneRefCost(b *testing.B) {