Enforce exclusive checker acquisition (#64543)

Author: jakebailey

tsc/internal/compiler/checkerpool.go

@@ -17,6 +17,8 @@ import (
 // request-scoped lifetime and reclamation. It returns a checker and a release
 // function that must be called when the caller is done with the checker.
 // The returned checker must not be accessed concurrently; each acquisition is exclusive.
+// Acquisitions are not reentrant, even when they share a request ID. Callers must
+// pass an already acquired checker to nested operations instead of acquiring again.
 // If file is non-nil, the pool may use it as an affinity hint to return the same
 // checker for the same file across calls.
 type CheckerPool interface {

tsc/internal/fourslash/tests/contentMapperInlayHints_test.go

@@ -0,0 +1,42 @@
+package fourslash_test
+
+import (
+	"testing"
+
+	"github.com/microsoft/TypeScript/tsc/internal/core"
+	"github.com/microsoft/TypeScript/tsc/internal/ls/lsutil"
+	"github.com/microsoft/TypeScript/tsc/internal/testutil"
+	"github.com/microsoft/TypeScript/tsc/internal/testutil/contentmappertest"
+)
+
+func TestContentMapperInlayHintsReleaseEachRange(t *testing.T) {
+	t.Parallel()
+	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
+	// The mapper emits the script verbatim followed by:
+	//
+	// function __render() {
+	//   void (value);
+	//   void (value);
+	//   void (value);
+	//   void (value);
+	// }
+	// export default {};
+	//
+	// The script and four template identifiers map to five disjoint virtual ranges.
+	// Each range must release its checker before processing the next range.
+	f, done := newContentMapperFourslash(t, `// @Filename: /app.vue
+<script>
+const value = () => 1;
+</script>
+{{value}}
+{{value}}
+{{value}}
+{{value}}
+`, contentmappertest.ComponentMapper, ".vue")
+	defer done()
+
+	f.GoToFile(t, "/app.vue")
+	f.VerifyBaselineInlayHints(t, nil, &lsutil.UserPreferences{
+		InlayHints: lsutil.InlayHintsPreferences{IncludeInlayVariableTypeHints: core.TSTrue},
+	})
+}

tsc/internal/ls/codeactions_fixclassincorrectlyimplementsinterface.go

@@ -67,6 +67,8 @@ func getCodeActionsToFixClassIncorrectlyImplementsInterface(context context.Cont
 }
 
 func getAllCodeActionsToFixClassIncorrectlyImplementsInterface(context context.Context, fixContext *CodeFixContext) (*CombinedCodeActions, error) {
+	allDiags := getAllDiagnostics(context, fixContext.Program, fixContext.SourceFile)
+
 	typeChecker, done := fixContext.Program.GetTypeCheckerForFile(context, fixContext.SourceFile)
 	defer done()
 
@@ -78,7 +80,7 @@ func getAllCodeActionsToFixClassIncorrectlyImplementsInterface(context context.C
 
 	seenClassDeclarations := collections.Set[*ast.Node]{}
 
-	for _, diag := range getAllDiagnostics(context, fixContext.Program, fixContext.SourceFile) {
+	for _, diag := range allDiags {
 		if isFixableDiagnostic(diag, fixClassIncorrectlyImplementsInterfaceErrorCodes) {
 			classDeclaration := getClass(fixContext.SourceFile, core.NewTextRange(diag.Pos(), diag.End()))
 			if classDeclaration == nil {

tsc/internal/ls/codeactions_fixmissingtypeannotation.go

@@ -126,6 +126,8 @@ func getIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *CodeFix
 }
 
 func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *CodeFixContext) (*CombinedCodeActions, error) {
+	allDiags := getAllDiagnostics(ctx, fixContext.Program, fixContext.SourceFile)
+
 	ch, done := fixContext.Program.GetTypeCheckerForFile(ctx, fixContext.SourceFile)
 	defer done()
 
@@ -141,7 +143,6 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code
 		typePrintMode: typePrintModeFull,
 	}
 
-	allDiags := getAllDiagnostics(ctx, fixContext.Program, fixContext.SourceFile)
 	for _, diag := range allDiags {
 		if isFixableDiagnostic(diag, isolatedDeclarationsFixErrorCodes) {
 			span := core.NewTextRange(diag.Loc().Pos(), diag.Loc().End())

tsc/internal/ls/codeactions_importfixes.go

@@ -61,7 +61,10 @@ type fixInfo struct {
 }
 
 func getImportCodeActions(ctx context.Context, fixContext *CodeFixContext) ([]*CodeAction, error) {
-	info, err := getFixInfos(ctx, fixContext, fixContext.ErrorCode, fixContext.Span.Pos())
+	ch, done := fixContext.Program.GetTypeChecker(ctx)
+	defer done()
+
+	info, err := getFixInfos(ch, fixContext, fixContext.ErrorCode, fixContext.Span.Pos())
 	if err != nil {
 		return nil, err
 	}
@@ -133,7 +136,7 @@ func getAllImportCodeActions(ctx context.Context, fixContext *CodeFixContext) (*
 	)
 
 	for _, diag := range importDiags {
-		if err := addImportFromDiagnostic(ctx, importAdder, diag, fixContext); err != nil {
+		if err := addImportFromDiagnostic(ch, importAdder, diag, fixContext); err != nil {
 			return nil, err
 		}
 	}
@@ -149,7 +152,7 @@ func getAllImportCodeActions(ctx context.Context, fixContext *CodeFixContext) (*
 }
 
 // addImportFromDiagnostic finds the best import fix for a diagnostic and adds it to the adder.
-func addImportFromDiagnostic(ctx context.Context, importAdder autoimport.ImportAdder, diag *ast.Diagnostic, fixContext *CodeFixContext) error {
+func addImportFromDiagnostic(ch *checker.Checker, importAdder autoimport.ImportAdder, diag *ast.Diagnostic, fixContext *CodeFixContext) error {
 	diagFixContext := &CodeFixContext{
 		SourceFile: fixContext.SourceFile,
 		Span:       core.NewTextRange(diag.Pos(), diag.End()),
@@ -158,7 +161,7 @@ func addImportFromDiagnostic(ctx context.Context, importAdder autoimport.ImportA
 		LS:         fixContext.LS,
 	}
 
-	infos, err := getFixInfos(ctx, diagFixContext, diag.Code(), diag.Pos())
+	infos, err := getFixInfos(ch, diagFixContext, diag.Code(), diag.Pos())
 	if err != nil {
 		return err
 	}
@@ -168,7 +171,7 @@ func addImportFromDiagnostic(ctx context.Context, importAdder autoimport.ImportA
 	return nil
 }
 
-func getFixInfos(ctx context.Context, fixContext *CodeFixContext, errorCode int32, pos int) ([]*fixInfo, error) {
+func getFixInfos(ch *checker.Checker, fixContext *CodeFixContext, errorCode int32, pos int) ([]*fixInfo, error) {
 	// Can't compute import fixes for dynamic/untitled files since they don't have real file paths
 	if tspath.IsDynamicFileName(fixContext.SourceFile.FileName()) {
 		return nil, nil
@@ -179,9 +182,6 @@ func getFixInfos(ctx context.Context, fixContext *CodeFixContext, errorCode int3
 		return nil, nil
 	}
 
-	ch, done := fixContext.Program.GetTypeChecker(ctx)
-	defer done()
-
 	var view *autoimport.View
 	var info []*fixInfo
 

tsc/internal/ls/findallreferences.go

@@ -1287,7 +1287,7 @@ func (l *LanguageService) getReferencedSymbolsForNode(ctx context.Context, posit
 		}
 
 		if moduleSymbol := checker.GetMergedSymbol(resolvedRef.file.Symbol); moduleSymbol != nil {
-			return l.getReferencedSymbolsForModule(ctx, program, moduleSymbol /*excludeImportTypeOfExportEquals*/, false, sourceFiles, sourceFilesSet)
+			return l.getReferencedSymbolsForModule(checker, program, moduleSymbol /*excludeImportTypeOfExportEquals*/, false, sourceFiles, sourceFilesSet)
 		}
 
 		// !!! not implemented
@@ -1336,7 +1336,7 @@ func (l *LanguageService) getReferencedSymbolsForNode(ctx context.Context, posit
 		if symbol.Parent == nil {
 			return nil
 		}
-		return l.getReferencedSymbolsForModule(ctx, program, symbol.Parent, false /*excludeImportTypeOfExportEquals*/, sourceFiles, sourceFilesSet)
+		return l.getReferencedSymbolsForModule(checker, program, symbol.Parent, false /*excludeImportTypeOfExportEquals*/, sourceFiles, sourceFilesSet)
 	}
 
 	moduleReferences := l.getReferencedSymbolsForModuleIfDeclaredBySourceFile(ctx, symbol, program, sourceFiles, checker, options, sourceFilesSet)
@@ -1410,7 +1410,7 @@ func (l *LanguageService) getReferencedSymbolsForModuleIfDeclaredBySourceFile(ct
 	}
 	exportEquals := symbol.Exports[ast.InternalSymbolNameExportEquals]
 	// If exportEquals != nil, we're about to add references to `import("mod")` anyway, so don't double-count them.
-	moduleReferences := l.getReferencedSymbolsForModule(ctx, program, symbol, exportEquals != nil, sourceFiles, sourceFilesSet)
+	moduleReferences := l.getReferencedSymbolsForModule(checker, program, symbol, exportEquals != nil, sourceFiles, sourceFilesSet)
 	if exportEquals == nil || exportEquals.Flags&ast.SymbolFlagsAlias == 0 || !sourceFilesSet.Has(moduleSourceFileName) {
 		return moduleReferences
 	}
@@ -1732,12 +1732,9 @@ func getMergedAliasedSymbolOfNamespaceExportDeclaration(node *ast.Node, symbol *
 	return nil
 }
 
-func (l *LanguageService) getReferencedSymbolsForModule(ctx context.Context, program *compiler.Program, symbol *ast.Symbol, excludeImportTypeOfExportEquals bool, sourceFiles []*ast.SourceFile, sourceFilesSet *collections.Set[string]) []*SymbolAndEntries {
+func (l *LanguageService) getReferencedSymbolsForModule(checker *checker.Checker, program *compiler.Program, symbol *ast.Symbol, excludeImportTypeOfExportEquals bool, sourceFiles []*ast.SourceFile, sourceFilesSet *collections.Set[string]) []*SymbolAndEntries {
 	debug.Assert(symbol.ValueDeclaration != nil)
 
-	checker, done := program.GetTypeChecker(ctx)
-	defer done()
-
 	moduleRefs := findModuleReferences(program, sourceFiles, symbol, checker)
 	references := core.MapNonNil(moduleRefs, func(reference ModuleReference) *ReferenceEntry {
 		switch reference.kind {

tsc/internal/ls/inlay_hints.go

@@ -38,20 +38,22 @@ func (l *LanguageService) ProvideInlayHint(
 	mappedRanges := l.converters.FromLSPRangeIntersectingForSourceFile(file, params.Range, spanmap.FeatureInlayHints)
 	result := make([]*lsproto.InlayHint, 0, len(mappedRanges))
 	for _, mapped := range mappedRanges {
-		projection := mapped.Script
-		checker, done := program.GetTypeCheckerForFile(ctx, projection)
-		defer done()
-		inlayHintState := &inlayHintState{
-			ctx:             ctx,
-			span:            mapped.Span,
-			preferences:     inlayHintPreferences,
-			quotePreference: quotePreference,
-			file:            projection,
-			checker:         checker,
-			converters:      l.converters,
-		}
-		inlayHintState.visit(projection.AsNode())
-		result = append(result, inlayHintState.result...)
+		func() {
+			projection := mapped.Script
+			checker, done := program.GetTypeCheckerForFile(ctx, projection)
+			defer done()
+			inlayHintState := &inlayHintState{
+				ctx:             ctx,
+				span:            mapped.Span,
+				preferences:     inlayHintPreferences,
+				quotePreference: quotePreference,
+				file:            projection,
+				checker:         checker,
+				converters:      l.converters,
+			}
+			inlayHintState.visit(projection.AsNode())
+			result = append(result, inlayHintState.result...)
+		}()
 	}
 	return lsproto.InlayHintsOrNull{InlayHints: &result}, nil
 }

tsc/internal/lsp/server_flakydiagnostics_test.go

@@ -0,0 +1,71 @@
+package lsp_test
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/microsoft/TypeScript/tsc/internal/bundled"
+	"github.com/microsoft/TypeScript/tsc/internal/lsp"
+	"github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto"
+	"github.com/microsoft/TypeScript/tsc/internal/testutil/lsptestutil"
+	"github.com/microsoft/TypeScript/tsc/internal/vfs/vfstest"
+	"gotest.tools/v3/assert"
+)
+
+func TestFlakyDiagnosticTrackingParallelEmit(t *testing.T) {
+	t.Parallel()
+	if !bundled.Embedded {
+		t.Skip("bundled files are not embedded")
+	}
+
+	for _, noEmitOnError := range []bool{false, true} {
+		t.Run(fmt.Sprintf("noEmitOnError=%t", noEmitOnError), func(t *testing.T) {
+			t.Parallel()
+			files := map[string]string{
+				"/src/tsconfig.json": fmt.Sprintf(`{
+					"compilerOptions": { "strict": true, "declaration": true, "noEmitOnError": %t, "outDir": "out" }
+				}`, noEmitOnError),
+				"/src/a.ts": `export function box<T>(value: T) { return { value }; }`,
+				"/src/b.ts": `import { box } from "./a"; export const b = box("b");`,
+				"/src/c.ts": `import { box } from "./a"; export const c = box(1);`,
+			}
+			client, closeClient := lsptestutil.NewLSPClient(t, lsp.ServerOptions{
+				Err:                io.Discard,
+				Cwd:                "/src",
+				FS:                 bundled.WrapFS(vfstest.FromMap(files, false)),
+				DefaultLibraryPath: bundled.LibPath(),
+			}, func(_ context.Context, req *lsproto.RequestMessage) *lsproto.ResponseMessage {
+				switch req.Method {
+				case lsproto.MethodClientRegisterCapability, lsproto.MethodClientUnregisterCapability, lsproto.MethodWindowWorkDoneProgressCreate:
+					return &lsproto.ResponseMessage{ID: req.ID, JSONRPC: req.JSONRPC, Result: lsproto.Null{}}
+				default:
+					return nil
+				}
+			})
+			t.Cleanup(func() { _ = closeClient() })
+
+			msg, _, ok := client.SendRequest(t, lsproto.InitializeInfo, &lsproto.InitializeParams{
+				Capabilities: &lsproto.ClientCapabilities{},
+				InitializationOptions: &lsproto.InitializationOptionsOrNull{
+					InitializationOptions: &lsproto.InitializationOptions{TrackFlakyDiagnostics: new(lsproto.DiagnosticFlakeLogLevelPanic)},
+				},
+			})
+			assert.Assert(t, ok && msg.AsResponse().Error == nil, "initialize failed")
+			client.SendNotification(t, lsproto.InitializedInfo, &lsproto.InitializedParams{})
+			<-client.Server.InitComplete()
+
+			uri := lsproto.DocumentUri("file:///src/a.ts")
+			client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{
+				TextDocument: &lsproto.TextDocumentItem{Uri: uri, LanguageId: lsproto.LanguageKindTypeScript, Text: files["/src/a.ts"]},
+			})
+			msg, diagnostics, ok := client.SendRequest(t, lsproto.TextDocumentDiagnosticInfo, &lsproto.DocumentDiagnosticParams{
+				TextDocument: lsproto.TextDocumentIdentifier{Uri: uri},
+			})
+			assert.Assert(t, ok && msg.AsResponse().Error == nil, "diagnostics request failed")
+			assert.Assert(t, diagnostics.FullDocumentDiagnosticReport != nil)
+			assert.Equal(t, len(diagnostics.FullDocumentDiagnosticReport.Items), 0)
+		})
+	}
+}

tsc/internal/project/checkerpool.go

@@ -142,76 +142,50 @@ func (p *checkerPool) GetChecker(ctx context.Context, file *ast.SourceFile) (*ch
 	}
 }
 
-// tryReacquireForRequest checks whether the given request already has an
-// associated checker. If so, it either returns the checker directly (still held)
-// or reacquires it by claiming a semaphore slot. The caller must provide the
+// tryReacquireForRequest claims a semaphore slot, then checks whether the given
+// request has an idle associated checker. The caller must provide the
 // appropriate semaphore channel and indicate whether this is a diagnostics
 // request (isDiag). If the associated checker is in the wrong category
 // (e.g. a diagnostics index for a query request), the association is deleted
 // and normal acquisition proceeds.
 //
-// Returns (checker, release, true) if the request was served (either still held
-// or reclaimed). Returns (nil, nil, false) if the caller must proceed with
+// Request affinity is only a preference for an idle checker, not permission to
+// reuse a held checker: concurrent acquisitions can share the same request ID.
+// Returns (checker, release, true) if the checker was reclaimed.
+// Returns (nil, nil, false) if the caller must proceed with
 // normal acquisition — in this case, a semaphore slot has already been claimed.
 // Must NOT be called with p.mu held.
 func (p *checkerPool) tryReacquireForRequest(requestID string, sem chan<- struct{}, isDiag bool) (*checker.Checker, func(), bool) {
+	sem <- struct{}{}
 	if requestID == "" {
-		sem <- struct{}{}
 		return nil, nil, false
 	}
 
 	p.mu.Lock()
+	defer p.mu.Unlock()
 	index, ok := p.requestAssociations[requestID]
 	if !ok {
-		p.mu.Unlock()
-		sem <- struct{}{}
 		return nil, nil, false
 	}
 
 	// Validate that the associated index matches the expected category.
 	// Index 0 is for diagnostics; indices 1+ are for queries.
 	if (isDiag && index != 0) || (!isDiag && index == 0) {
 		delete(p.requestAssociations, requestID)
-		p.mu.Unlock()
-		sem <- struct{}{}
 		return nil, nil, false
 	}
 
 	c := p.checkers[index]
 	if c == nil {
 		delete(p.requestAssociations, requestID)
-		p.mu.Unlock()
-		sem <- struct{}{}
 		return nil, nil, false
 	}
 
-	held := p.heldBy[index]
-	if held == requestID {
-		// Same request, checker still held — return without claiming a slot.
-		p.mu.Unlock()
-		return c, noop, true
-	}
-
-	if held == "" {
-		// Same request reacquiring after release — need a semaphore slot.
-		p.mu.Unlock()
-		sem <- struct{}{}
-		p.mu.Lock()
-		// Re-check: checker may have been disposed while waiting for the slot.
-		if cc := p.checkers[index]; cc == c && p.heldBy[index] == "" {
-			p.heldBy[index] = requestID
-			p.mu.Unlock()
-			return c, p.createRelease(requestID, index, c), true
-		}
-		p.mu.Unlock()
-		// Checker was replaced/disposed while waiting for the slot.
-		// The slot is still claimed; the caller will use it for normal acquisition.
-		return nil, nil, false
+	if p.heldBy[index] == "" {
+		p.heldBy[index] = requestID
+		return c, p.createRelease(requestID, index, c), true
 	}
 
-	// Checker held by another request — claim a slot normally.
-	p.mu.Unlock()
-	sem <- struct{}{}
 	return nil, nil, false
 }
 
@@ -529,5 +503,3 @@ func (p *checkerPool) Discard() {
 		p.cleanupTimer = nil
 	}
 }
-
-func noop() {}