Don't let parallel per-file workers reuse their request's checker

tsc/internal/compiler/program.go

@@ -675,12 +675,26 @@ func (p *Program) collectDiagnostics(ctx context.Context, sourceFile *ast.Source
 	return filterAndSortDiagnostics(result)
 }
 
+// getWorkerContext returns the context for the per-file workers of a whole-program operation.
+// Workers that run on their own goroutines must not act as the request itself,
+// or the checker pool would let all of them reuse the one checker the request
+// holds at the same time (see core.WithoutRequestID). Single-threaded workers
+// run on the caller's goroutine, so they keep the request's identity.
+func getWorkerContext(ctx context.Context, singleThreaded bool) context.Context {
+	if singleThreaded {
+		return ctx
+	}
+	return core.WithoutRequestID(ctx)
+}
+
 func (p *Program) collectDiagnosticsFromFiles(ctx context.Context, sourceFiles []*ast.SourceFile, concurrent bool, collect func(context.Context, *ast.SourceFile) []*ast.Diagnostic) [][]*ast.Diagnostic {
 	diagnostics := make([][]*ast.Diagnostic, len(sourceFiles))
-	wg := core.NewWorkGroup(!concurrent || p.SingleThreaded())
+	singleThreaded := !concurrent || p.SingleThreaded()
+	wg := core.NewWorkGroup(singleThreaded)
+	workerCtx := getWorkerContext(ctx, singleThreaded)
 	for i, file := range sourceFiles {
 		wg.Queue(func() {
-			diagnostics[i] = collect(ctx, file)
+			diagnostics[i] = collect(workerCtx, file)
 		})
 	}
 	wg.RunAndWait()
@@ -725,13 +739,14 @@ func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, source
 		})
 	} else {
 		wg := core.NewWorkGroup(p.SingleThreaded())
+		workerCtx := getWorkerContext(ctx, p.SingleThreaded())
 		for i, file := range sourceFiles {
 			if p.SkipTypeChecking(file, false) {
 				continue
 			}
 			wg.Queue(func() {
-				c, done := p.checkerPool.GetChecker(ctx, file)
-				diagnostics[i] = collect(ctx, c, file)
+				c, done := p.checkerPool.GetChecker(workerCtx, file)
+				diagnostics[i] = collect(workerCtx, c, file)
 				done()
 			})
 		}
@@ -1892,6 +1907,7 @@ func (p *Program) Emit(ctx context.Context, options EmitOptions) *EmitResult {
 	forceDtsEmit := options.EmitOnly == EmitOnlyBuilderSignature || options.ForceEmit && options.EmitOnly == EmitOnlyDts
 	forceJsEmit := options.ForceEmit && options.EmitOnly == EmitOnlyJs
 	sourceFiles := p.getSourceFilesToEmit(options.TargetSourceFiles, forceDtsEmit, forceJsEmit)
+	workerCtx := getWorkerContext(ctx, p.SingleThreaded())
 
 	for _, sourceFile := range sourceFiles {
 		emitter := &emitter{
@@ -1904,7 +1920,7 @@ func (p *Program) Emit(ctx context.Context, options EmitOptions) *EmitResult {
 		}
 		emitters = append(emitters, emitter)
 		wg.Queue(func() {
-			host, done := newEmitHost(ctx, p, sourceFile)
+			host, done := newEmitHost(workerCtx, p, sourceFile)
 			defer done()
 			emitter.host = host
 

tsc/internal/core/context.go

@@ -15,6 +15,19 @@ func WithRequestID(ctx context.Context, id string) context.Context {
 	return context.WithValue(ctx, requestIDKey, id)
 }
 
+// WithoutRequestID returns a context for one of several workers that a request
+// runs in parallel, such as one per file. Checker pools use the request ID to hand a
+// request back a checker it already holds (nested acquisition), which is only
+// sound while all of a request's acquisitions happen on a single goroutine.
+// Workers that run concurrently with each other must therefore acquire their
+// checkers as independent callers rather than as the request itself.
+func WithoutRequestID(ctx context.Context) context.Context {
+	if GetRequestID(ctx) == "" {
+		return ctx
+	}
+	return context.WithValue(ctx, requestIDKey, "")
+}
+
 func GetRequestID(ctx context.Context) string {
 	if id, ok := ctx.Value(requestIDKey).(string); ok {
 		return id

tsc/internal/lsp/server_flakydiagnostics_test.go

@@ -0,0 +1,83 @@
+package lsp_test
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"strings"
+	"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"
+)
+
+// With flaky diagnostic tracking on, a diagnostics request also emits the whole
+// project, and with noEmitOnError that first checks every file in parallel. Those
+// parallel checks used to share one checker and crash the server.
+func TestFlakyDiagnosticTrackingWithNoEmitOnError(t *testing.T) {
+	t.Parallel()
+	if !bundled.Embedded {
+		t.Skip("bundled files are not embedded")
+	}
+
+	// Enough files with enough to check that the parallel checks overlap.
+	files := map[string]string{
+		"/home/projects/tsconfig.json": `{ "compilerOptions": { "strict": true, "noEmitOnError": true, "declaration": true, "outDir": "out" } }`,
+	}
+	for i := range 12 {
+		var b strings.Builder
+		if i > 0 {
+			fmt.Fprintf(&b, "import { box0 } from \"./file%d\";\nexport const imported = box0.map(x => x);\n", i-1)
+		}
+		for j := range 20 {
+			fmt.Fprintf(&b, `
+export interface Box%[1]d<T> { value: T; map<U>(f: (x: T) => U): Box%[1]d<U> }
+export declare const box%[1]d: Box%[1]d<string>;
+export const mapped%[1]d = box%[1]d.map(s => s.length).map(n => ({ n, even: n %% 2 === 0 }));
+`, j)
+		}
+		files[fmt.Sprintf("/home/projects/file%d.ts", i)] = b.String()
+	}
+
+	onServerRequest := 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
+		}
+	}
+	client, closeClient := lsptestutil.NewLSPClient(t, lsp.ServerOptions{
+		Err:                io.Discard,
+		Cwd:                "/home/projects",
+		FS:                 bundled.WrapFS(vfstest.FromMap(files, false)),
+		DefaultLibraryPath: bundled.LibPath(),
+	}, onServerRequest)
+	t.Cleanup(func() { _ = closeClient() })
+
+	initMsg, _, 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 && initMsg.AsResponse().Error == nil, "Initialize failed")
+	client.SendNotification(t, lsproto.InitializedInfo, &lsproto.InitializedParams{})
+	<-client.Server.InitComplete()
+
+	uri := lsproto.DocumentUri("file:///home/projects/file0.ts")
+	client.SendNotification(t, lsproto.TextDocumentDidOpenInfo, &lsproto.DidOpenTextDocumentParams{
+		TextDocument: &lsproto.TextDocumentItem{Uri: uri, LanguageId: lsproto.LanguageKindTypeScript, Text: files["/home/projects/file0.ts"]},
+	})
+
+	diagnosticMsg, diagnostics, ok := client.SendRequest(t, lsproto.TextDocumentDiagnosticInfo, &lsproto.DocumentDiagnosticParams{
+		TextDocument: lsproto.TextDocumentIdentifier{Uri: uri},
+	})
+	assert.Assert(t, ok && diagnosticMsg.AsResponse().Error == nil, "diagnostics request failed")
+	assert.Assert(t, diagnostics.FullDocumentDiagnosticReport != nil)
+	assert.Equal(t, len(diagnostics.FullDocumentDiagnosticReport.Items), 0)
+}