From 9a88eb9ccb4e0020115d2a240ec96dc3abbde5d9 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 10 Jul 2026 12:14:06 -0700 Subject: [PATCH 01/14] Honor context cancellation in one-shot tsc compile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI wires SIGINT/SIGTERM to a context in cmd/tsgo/main.go, but the plain (non-watch, non-build) compile path never threaded it to the checker: performCompilation/performIncrementalCompilation didn't accept a context and EmitFilesAndReportErrors hardcoded context.Background(). As a result a large `--noEmit` compile ignored Ctrl-C entirely — and because signal.NotifyContext replaces the default handler, the process wouldn't even die, it ran to completion. Thread the context through the compile path so the checker's existing cooperative cancellation polling (isCanceled) takes effect, and add a distinct ExitStatusCanceled (6) returned when the compile is aborted so incomplete diagnostics are not reported as a complete result. Introducing cancellation into the compiler checker pool exposed a latent panic: the pool reuses a fixed set of checkers across the diagnostics pass, and a canceled checker panics on reuse (checkNotCanceled). Guard the reuse sites (the second GetGlobalDiagnostics pass and the per-file checker-group loop) with ctx.Err() so a canceled checker is not fed more work. These guards are only necessary because the diagnostics APIs return a bare []*ast.Diagnostic with no error channel, making an empty (canceled) result indistinguishable from a clean one; the comments record that root cause. Watcher and build-task emit call sites pass context.Background() to preserve current behavior (their cancellation is handled at the RunLoop / orchestrator level). Add TestTscNoEmitCancellation covering both the single-file short-circuit and the multi-file/single-checker reuse path that would otherwise panic. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/compiler/checkerpool.go | 12 +++ internal/compiler/program.go | 13 +++ internal/execute/build/buildtask.go | 5 +- internal/execute/tsc.go | 8 +- internal/execute/tsc/compile.go | 1 + internal/execute/tsc/emit.go | 15 +++- internal/execute/tsctests/runner.go | 2 + internal/execute/tsctests/tsccancel_test.go | 87 +++++++++++++++++++++ internal/execute/watcher.go | 4 +- 9 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 internal/execute/tsctests/tsccancel_test.go diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index 55158370765..1f36f20f2a6 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -155,6 +155,18 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So p.locks[checkerIdx].Lock() defer p.locks[checkerIdx].Unlock() for i, file := range files { + // Once the context is canceled, the checker enters a canceled state and + // reusing it for the next file would panic in checkNotCanceled. Stop early. + // + // This guard is only necessary because the diagnostics APIs return a bare + // []*ast.Diagnostic with no error channel: a canceled check yields an empty + // (incomplete) slice that is indistinguishable from a clean result, so + // cancellation is signaled out-of-band via the checker's canceled state + // rather than a returned error. If those APIs returned (diags, error), the + // caller would stop on the error and this guard would be unnecessary. + if ctx.Err() != nil { + break + } if checker := p.checkers[checkerIdx]; checker == p.fileAssociations[file] { cb(checker, i, file) } diff --git a/internal/compiler/program.go b/internal/compiler/program.go index 3299b6a3acb..27563fd31fc 100644 --- a/internal/compiler/program.go +++ b/internal/compiler/program.go @@ -613,6 +613,11 @@ func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, source continue } wg.Queue(func() { + // A checker obtained from the pool may already be in a canceled state from a + // prior file; reusing it would panic in checkNotCanceled. Skip once canceled. + if ctx.Err() != nil { + return + } c, done := p.checkerPool.GetChecker(ctx, file) diagnostics[i] = collect(ctx, c, file) done() @@ -1815,6 +1820,14 @@ func GetDiagnosticsOfAnyProgram( if len(allDiagnostics) == configFileParsingDiagnosticsLength { allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics) + // If checking was canceled, the checker is now in a canceled state and must not + // be reused (GetGlobalDiagnostics/GetDeclarationDiagnostics would panic in + // checkNotCanceled). The diagnostics gathered so far are incomplete and will be + // discarded by the caller, so stop here. See checkerPool.forEachCheckerGroupDo + // for why this out-of-band check is needed (diagnostics APIs have no error channel). + if ctx.Err() != nil { + return allDiagnostics + } // Ask for the global diagnostics again (they were empty above); we may have found new during checking, e.g. missing globals. allDiagnostics = append(allDiagnostics, program.GetGlobalDiagnostics(ctx)...) } diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 3508d2de074..4211e795d59 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -1,6 +1,7 @@ package build import ( + "context" "fmt" "slices" "strings" @@ -216,7 +217,9 @@ func (t *BuildTask) compileAndEmit(orchestrator *Orchestrator, path tspath.Path) t.result.program = incremental.NewProgram(program, oldProgram, orchestrator.host, orchestrator.opts.Sys.Now, orchestrator.opts.Testing != nil) compileTimes.ChangesComputeTime = orchestrator.opts.Sys.Now().Sub(changesComputeStart) - result, statistics := tsc.EmitAndReportStatistics(tsc.EmitInput{ + // The build orchestrator does not thread a per-task context today; cancellation + // for `tsc -b` is handled at the orchestrator level. + result, statistics := tsc.EmitAndReportStatistics(context.Background(), tsc.EmitInput{ Sys: orchestrator.opts.Sys, ProgramLike: t.result.program, Program: program, diff --git a/internal/execute/tsc.go b/internal/execute/tsc.go index 7db8be97a02..07587bc64a7 100644 --- a/internal/execute/tsc.go +++ b/internal/execute/tsc.go @@ -243,6 +243,7 @@ func tscCompilation(ctx context.Context, sys tsc.System, commandLine *tsoptions. return tsc.CommandLineResult{Status: tsc.ExitStatusSuccess, Watcher: watcher} } else if configForCompilation.CompilerOptions().IsIncremental() { return performIncrementalCompilation( + ctx, sys, configForCompilation, reportDiagnostic, @@ -253,6 +254,7 @@ func tscCompilation(ctx context.Context, sys tsc.System, commandLine *tsoptions. ) } return performCompilation( + ctx, sys, configForCompilation, reportDiagnostic, @@ -282,6 +284,7 @@ func getTraceFromSys(sys tsc.System, locale locale.Locale, testing tsc.CommandLi } func performIncrementalCompilation( + ctx context.Context, sys tsc.System, config *tsoptions.ParsedCommandLine, reportDiagnostic tsc.DiagnosticReporter, @@ -307,7 +310,7 @@ func performIncrementalCompilation( changesComputeStart := sys.Now() incrementalProgram := incremental.NewProgram(program, oldProgram, incremental.CreateHost(host), sys.Now, testing != nil) compileTimes.ChangesComputeTime = sys.Now().Sub(changesComputeStart) - result, _ := tsc.EmitAndReportStatistics(tsc.EmitInput{ + result, _ := tsc.EmitAndReportStatistics(ctx, tsc.EmitInput{ Sys: sys, ProgramLike: incrementalProgram, Program: incrementalProgram.GetProgram(), @@ -331,6 +334,7 @@ func performIncrementalCompilation( } func performCompilation( + ctx context.Context, sys tsc.System, config *tsoptions.ParsedCommandLine, reportDiagnostic tsc.DiagnosticReporter, @@ -350,7 +354,7 @@ func performCompilation( Tracing: tr, }) compileTimes.ParseTime = sys.Now().Sub(parseStart) - result, _ := tsc.EmitAndReportStatistics(tsc.EmitInput{ + result, _ := tsc.EmitAndReportStatistics(ctx, tsc.EmitInput{ Sys: sys, ProgramLike: program, Program: program, diff --git a/internal/execute/tsc/compile.go b/internal/execute/tsc/compile.go index 4f69345efaa..aa771bd0185 100644 --- a/internal/execute/tsc/compile.go +++ b/internal/execute/tsc/compile.go @@ -36,6 +36,7 @@ const ( ExitStatusInvalidProject_OutputsSkipped ExitStatus = 3 ExitStatusProjectReferenceCycle_OutputsSkipped ExitStatus = 4 ExitStatusNotImplemented ExitStatus = 5 + ExitStatusCanceled ExitStatus = 6 ) type Watcher interface { diff --git a/internal/execute/tsc/emit.go b/internal/execute/tsc/emit.go index 88e03f7a19e..6a8f2d1de60 100644 --- a/internal/execute/tsc/emit.go +++ b/internal/execute/tsc/emit.go @@ -43,9 +43,9 @@ type EmitInput struct { Tracing *tracing.Tracing } -func EmitAndReportStatistics(input EmitInput) (CompileAndEmitResult, *Statistics) { +func EmitAndReportStatistics(ctx context.Context, input EmitInput) (CompileAndEmitResult, *Statistics) { var statistics *Statistics - result := EmitFilesAndReportErrors(input) + result := EmitFilesAndReportErrors(ctx, input) if result.Status != ExitStatusSuccess { // compile exited early return result, nil @@ -71,9 +71,8 @@ func EmitAndReportStatistics(input EmitInput) (CompileAndEmitResult, *Statistics return result, statistics } -func EmitFilesAndReportErrors(input EmitInput) (result CompileAndEmitResult) { +func EmitFilesAndReportErrors(ctx context.Context, input EmitInput) (result CompileAndEmitResult) { result.times = input.CompileTimes - ctx := context.Background() allDiagnostics := compiler.GetDiagnosticsOfAnyProgram( ctx, @@ -112,6 +111,14 @@ func EmitFilesAndReportErrors(input EmitInput) (result CompileAndEmitResult) { }, ) + // If the compile was canceled (e.g. SIGINT), the checker stops early and the + // diagnostics above are incomplete. Do not emit or report them as a complete + // result; abort with a distinct status instead. + if ctx.Err() != nil { + result.Status = ExitStatusCanceled + return result + } + emitResult := &compiler.EmitResult{EmitSkipped: true, Diagnostics: []*ast.Diagnostic{}} if !input.ProgramLike.Options().ListFilesOnly.IsTrue() { emitStart := input.Sys.Now() diff --git a/internal/execute/tsctests/runner.go b/internal/execute/tsctests/runner.go index 38c7e9c2089..7653905cbf2 100644 --- a/internal/execute/tsctests/runner.go +++ b/internal/execute/tsctests/runner.go @@ -57,6 +57,8 @@ func (test *tscInput) executeCommand(sys *TestSys, baselineBuilder *strings.Buil baselineBuilder.WriteString("ExitStatus:: ProjectReferenceCycle_OutputsSkipped") case tsc.ExitStatusNotImplemented: baselineBuilder.WriteString("ExitStatus:: NotImplemented") + case tsc.ExitStatusCanceled: + baselineBuilder.WriteString("ExitStatus:: Canceled") default: panic(fmt.Sprintf("UnknownExitStatus %d", result.Status)) } diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go new file mode 100644 index 00000000000..b0d53fe601b --- /dev/null +++ b/internal/execute/tsctests/tsccancel_test.go @@ -0,0 +1,87 @@ +package tsctests + +import ( + "context" + "strings" + "testing" + + "github.com/microsoft/typescript-go/internal/execute" + "github.com/microsoft/typescript-go/internal/execute/tsc" +) + +// TestTscNoEmitCancellation verifies that interrupting a `--noEmit` compile via +// the context passed to execute.CommandLine (which the CLI wires to SIGINT/SIGTERM +// in cmd/tsgo/main.go) aborts the compile promptly rather than running it to +// completion. +// +// A pre-canceled context deterministically exercises the same isCanceled() polling +// the checker uses for a mid-flight SIGINT (see internal/checker/utilities.go), so +// it covers both the "already canceled" and "canceled during check" cases. +func TestTscNoEmitCancellation(t *testing.T) { + t.Parallel() + + // Each file contains a type error so we can prove whether the checker ran to + // completion: if it did, the diagnostic is reported; if the compile was + // abandoned on cancellation, it is not. + const badSource = `const x: number = "not a number";` + + testCases := []struct { + name string + args []string + files FileMap + }{ + { + // Single file: exercises the top-level cancellation short-circuit in + // EmitFilesAndReportErrors / GetDiagnosticsOfAnyProgram. + name: "single file", + args: []string{"--noEmit"}, + files: FileMap{ + "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "noEmit": true, "strict": true } }`, + "/home/src/workspaces/project/main.ts": badSource, + }, + }, + { + // Multiple files under --singleThreaded funnel through a single checker, + // so the per-file loop in checkerPool.forEachCheckerGroupDo reuses the + // same checker across files. Once the first file cancels it, reusing it + // for the next file would panic in checkNotCanceled without the guard + // there. This case pins that guard. + name: "multi file single checker", + args: []string{"--noEmit", "--singleThreaded"}, + files: FileMap{ + "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "noEmit": true, "strict": true } }`, + "/home/src/workspaces/project/a.ts": badSource, + "/home/src/workspaces/project/b.ts": badSource, + "/home/src/workspaces/project/c.ts": badSource, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + sys := newTestSys(&tscInput{ + commandLineArgs: tc.args, + files: tc.files, + }, false) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // simulate SIGINT delivered before/at the start of the compile + + result := execute.CommandLine(ctx, sys, tc.args, sys) + + // The compile should short-circuit with a distinct canceled status + // instead of running the checker to completion (and it must not panic + // reusing a canceled checker). + if result.Status != tsc.ExitStatusCanceled { + t.Errorf("status = %v, want ExitStatusCanceled (compile should abort on cancellation)", result.Status) + } + + // Because the check was abandoned, its (incomplete) diagnostics must not + // be reported: the type errors should be absent from the output. + if out := sys.getOutput(true); strings.Contains(out, "error TS") { + t.Errorf("expected no diagnostics to be reported after cancellation; got output:\n%s", out) + } + }) + } +} diff --git a/internal/execute/watcher.go b/internal/execute/watcher.go index af2aaa481c7..07e8f096ed5 100644 --- a/internal/execute/watcher.go +++ b/internal/execute/watcher.go @@ -527,7 +527,9 @@ func (w *Watcher) evictChangedSourceFiles(changedPaths map[string]fswatch.EventK } func (w *Watcher) compileAndEmit() tsc.CompileAndEmitResult { - return tsc.EmitFilesAndReportErrors(tsc.EmitInput{ + // Watch-cycle cancellation is handled at the RunLoop level (see WatchManager.RunLoop), + // so the per-cycle compile runs to completion. + return tsc.EmitFilesAndReportErrors(context.Background(), tsc.EmitInput{ Sys: w.sys, ProgramLike: w.program, Program: w.program.GetProgram(), From 738d3b2681397703d6a80e8c250874e6cc910768 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 10 Jul 2026 13:30:15 -0700 Subject: [PATCH 02/14] Honor context cancellation in tsc -b (non-watch build) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orchestrator.Start received the signal context but ran the actual non-watch build via buildOrClean() without it — ctx only reached the watch loop. So a plain `tsc -b` never consulted cancellation: SIGINT was captured (default handler suppressed by signal.NotifyContext) yet the build ran to completion. Same class of bug as the one-shot compile. Thread ctx through Start -> buildOrClean -> rangeTask -> buildOrCleanProject -> buildProject -> compileAndEmit -> EmitAndReportStatistics so a long build responds to SIGINT at the checker's granularity. rangeTask stops scheduling further projects once canceled; buildOrClean reports ExitStatusCanceled even if cancellation lands before any project produces a status; compileAndEmit early-returns on a canceled result (its EmitResult is nil) without updating timestamps or marking the project up-to-date. The canceled status dominates the aggregate via max. Watch mode is unchanged: the initial build and each DoCycle run to completion with context.Background(), cancellation observed at the RunLoop boundary. Also switch the compiler checker-group reuse guard to also check checker.WasCanceled() (the actual reuse precondition) and add a build-mode case to TestTscNoEmitCancellation. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/compiler/checkerpool.go | 26 +++++++----- internal/execute/build/buildtask.go | 18 +++++--- internal/execute/build/orchestrator.go | 46 +++++++++++++++------ internal/execute/tsctests/tsccancel_test.go | 12 ++++++ internal/execute/watcher.go | 11 ++++- 5 files changed, 82 insertions(+), 31 deletions(-) diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index 1f36f20f2a6..be66fe1ac8f 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -155,19 +155,25 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So p.locks[checkerIdx].Lock() defer p.locks[checkerIdx].Unlock() for i, file := range files { - // Once the context is canceled, the checker enters a canceled state and - // reusing it for the next file would panic in checkNotCanceled. Stop early. + checker := p.checkers[checkerIdx] + // Stop feeding this checker once cancellation is in play: ctx.Err() + // means nothing we produce will be used, and WasCanceled() means the + // checker is already poisoned so reusing it would panic in + // checkNotCanceled. The two coincide in the compile path today (one + // context, single-use pool), but WasCanceled() states the actual reuse + // precondition rather than a proxy for it, and guards against a checker + // canceled by some other context. // - // This guard is only necessary because the diagnostics APIs return a bare - // []*ast.Diagnostic with no error channel: a canceled check yields an empty - // (incomplete) slice that is indistinguishable from a clean result, so - // cancellation is signaled out-of-band via the checker's canceled state - // rather than a returned error. If those APIs returned (diags, error), the - // caller would stop on the error and this guard would be unnecessary. - if ctx.Err() != nil { + // This out-of-band check is only necessary because the diagnostics APIs + // return a bare []*ast.Diagnostic with no error channel: a canceled check + // yields an empty (incomplete) slice indistinguishable from a clean result, + // so cancellation is signaled via checker state rather than a returned error. + // If those APIs returned (diags, error), the caller would stop on the error + // and this guard would be unnecessary. + if ctx.Err() != nil || checker.WasCanceled() { break } - if checker := p.checkers[checkerIdx]; checker == p.fileAssociations[file] { + if checker == p.fileAssociations[file] { cb(checker, i, file) } } diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 4211e795d59..49a0fe45107 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -121,14 +121,14 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b close(t.reportDone) } -func (t *BuildTask) buildProject(orchestrator *Orchestrator, path tspath.Path) { +func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { // Wait on upstream tasks to complete t.waitOnUpstream() if t.pending.Load() { t.status = t.getUpToDateStatus(orchestrator, path) t.reportUpToDateStatus(orchestrator) if !t.handleStatusThatDoesntRequireBuild(orchestrator) { - t.compileAndEmit(orchestrator, path) + t.compileAndEmit(ctx, orchestrator, path) t.updateDownstream(orchestrator, path) } else { if t.resolved != nil { @@ -188,7 +188,7 @@ func (t *BuildTask) updateDownstream(orchestrator *Orchestrator, path tspath.Pat } } -func (t *BuildTask) compileAndEmit(orchestrator *Orchestrator, path tspath.Path) { +func (t *BuildTask) compileAndEmit(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { t.errors = nil if orchestrator.opts.Command.BuildOptions.Verbose.IsTrue() { t.result.reportStatus(ast.NewCompilerDiagnostic(diagnostics.Building_project_0, orchestrator.relativeFileName(t.config))) @@ -217,9 +217,7 @@ func (t *BuildTask) compileAndEmit(orchestrator *Orchestrator, path tspath.Path) t.result.program = incremental.NewProgram(program, oldProgram, orchestrator.host, orchestrator.opts.Sys.Now, orchestrator.opts.Testing != nil) compileTimes.ChangesComputeTime = orchestrator.opts.Sys.Now().Sub(changesComputeStart) - // The build orchestrator does not thread a per-task context today; cancellation - // for `tsc -b` is handled at the orchestrator level. - result, statistics := tsc.EmitAndReportStatistics(context.Background(), tsc.EmitInput{ + result, statistics := tsc.EmitAndReportStatistics(ctx, tsc.EmitInput{ Sys: orchestrator.opts.Sys, ProgramLike: t.result.program, Program: program, @@ -236,6 +234,14 @@ func (t *BuildTask) compileAndEmit(orchestrator *Orchestrator, path tspath.Path) }) t.result.exitStatus = result.Status t.result.statistics = statistics + if result.Status == tsc.ExitStatusCanceled { + // The compile was canceled (e.g. SIGINT). Its result is incomplete + // (result.EmitResult is nil), so do not update output timestamps or mark the + // project up-to-date, and leave buildKind as None so it is not counted as + // built. report() propagates the canceled exit status (it dominates via max), + // and rangeTask stops scheduling further projects. + return + } t.packageJsons = t.result.program.PackageJsonLookupPaths() if (!program.Options().NoEmitOnError.IsTrue() || len(result.Diagnostics) == 0) && (len(result.EmitResult.EmittedFiles) > 0 || t.status.kind != upToDateStatusTypeOutOfDateBuildInfoWithErrors) { diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index da13e64bbff..99aa9bceeef 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -229,12 +229,16 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { o.watchStatusReporter(ast.NewCompilerDiagnostic(diagnostics.Starting_compilation_in_watch_mode)) } o.GenerateGraph(nil) - result := o.buildOrClean() if o.opts.Command.CompilerOptions.Watch.IsTrue() { + // In watch mode the initial build, like each watch cycle, runs to completion; + // cancellation is observed at the RunLoop boundary (see the TODO in DoCycle). + result := o.buildOrClean(context.Background()) o.Watch(ctx) result.Watcher = o + return result } - return result + // Non-watch `tsc -b`: honor cancellation so a long build responds to SIGINT. + return o.buildOrClean(ctx) } func (o *Orchestrator) Watch(ctx context.Context) { @@ -265,7 +269,7 @@ func (o *Orchestrator) Watch(ctx context.Context) { func (o *Orchestrator) updateWatch() { oldCache := o.host.mTimes o.host.mTimes = &collections.SyncMap[tspath.Path, time.Time]{} - o.rangeTask(func(path tspath.Path, task *BuildTask) { + o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { task.updateWatch(o, oldCache) }) } @@ -390,7 +394,7 @@ func (o *Orchestrator) checkTasksForEventChanges(changedPaths map[string]fswatch for eventPath := range changedPaths { if o.host.FS().DirectoryExists(eventPath) { if o.wm.IsPathUnderWatch(eventPath, opts) { - o.rangeTask(func(path tspath.Path, task *BuildTask) { + o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { task.resetStatus() task.reportDone = make(chan struct{}) task.done = make(chan struct{}) @@ -542,7 +546,7 @@ func (o *Orchestrator) DoCycle() { if overflow { // Overflow: reset all tasks to force a full rebuild. - o.rangeTask(func(path tspath.Path, task *BuildTask) { + o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { task.resetConfig(o, path) task.reportDone = make(chan struct{}) task.done = make(chan struct{}) @@ -565,7 +569,11 @@ func (o *Orchestrator) DoCycle() { o.GenerateGraphReusingOldTasks() } - o.buildOrClean() + // TODO: like the CLI watcher, a build watch cycle runs to completion; cancellation + // is only observed between cycles in WatchManager.RunLoop. Thread the RunLoop context + // through DoCycle to make a long rebuild interruptible (and handle ExitStatusCanceled + // by discarding the partial cycle). + o.buildOrClean(context.Background()) o.updateWatch() desiredDirs := o.computeDesiredWatches() if err := o.wm.ReconcileWatches(desiredDirs); err != nil { @@ -576,7 +584,7 @@ func (o *Orchestrator) DoCycle() { o.resetCaches() } -func (o *Orchestrator) buildOrClean() tsc.CommandLineResult { +func (o *Orchestrator) buildOrClean(ctx context.Context) tsc.CommandLineResult { if !o.opts.Command.BuildOptions.Clean.IsTrue() && o.opts.Command.BuildOptions.Verbose.IsTrue() { o.createBuilderStatusReporter(nil)(ast.NewCompilerDiagnostic( diagnostics.Projects_in_this_build_Colon_0, @@ -588,9 +596,15 @@ func (o *Orchestrator) buildOrClean() tsc.CommandLineResult { var buildResult orchestratorResult if len(o.errors) == 0 { buildResult.statistics.Projects = len(o.Order()) - o.rangeTask(func(path tspath.Path, task *BuildTask) { - o.buildOrCleanProject(task, path, &buildResult) + o.rangeTask(ctx, func(ctx context.Context, path tspath.Path, task *BuildTask) { + o.buildOrCleanProject(ctx, task, path, &buildResult) }) + if ctx.Err() != nil { + // The build was canceled (e.g. SIGINT). rangeTask stops scheduling further + // projects; report the aborted status even if cancellation landed before any + // project produced one. + buildResult.result.Status = tsc.ExitStatusCanceled + } } else { // Circularity errors prevent any project from being built buildResult.result.Status = tsc.ExitStatusProjectReferenceCycle_OutputsSkipped @@ -604,7 +618,7 @@ func (o *Orchestrator) buildOrClean() tsc.CommandLineResult { return buildResult.result } -func (o *Orchestrator) rangeTask(f func(path tspath.Path, task *BuildTask)) { +func (o *Orchestrator) rangeTask(ctx context.Context, f func(ctx context.Context, path tspath.Path, task *BuildTask)) { numRoutines := 4 if o.opts.Command.CompilerOptions.SingleThreaded.IsTrue() { numRoutines = 1 @@ -625,7 +639,13 @@ func (o *Orchestrator) rangeTask(f func(path tspath.Path, task *BuildTask)) { } runTask := func() { for path, task, ok := getNextTask(); ok; path, task, ok = getNextTask() { - f(path, task) + // Stop scheduling further projects once canceled (e.g. SIGINT). Tasks + // already in flight finish on their own; the compile they run is + // interruptible via the context threaded into compileAndEmit. + if ctx.Err() != nil { + return + } + f(ctx, path, task) } } @@ -640,12 +660,12 @@ func (o *Orchestrator) rangeTask(f func(path tspath.Path, task *BuildTask)) { } } -func (o *Orchestrator) buildOrCleanProject(task *BuildTask, path tspath.Path, buildResult *orchestratorResult) { +func (o *Orchestrator) buildOrCleanProject(ctx context.Context, task *BuildTask, path tspath.Path, buildResult *orchestratorResult) { task.result = &taskResult{} task.result.reportStatus = o.createBuilderStatusReporter(task) task.result.diagnosticReporter = o.createDiagnosticReporter(task) if !o.opts.Command.BuildOptions.Clean.IsTrue() { - task.buildProject(o, path) + task.buildProject(ctx, o, path) } else { task.cleanProject(o, path) } diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index b0d53fe601b..c31e606a82f 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -55,6 +55,18 @@ func TestTscNoEmitCancellation(t *testing.T) { "/home/src/workspaces/project/c.ts": badSource, }, }, + { + // Build mode (`tsc -b`): exercises the context threaded through the build + // orchestrator (Start -> buildOrClean -> rangeTask -> buildProject -> + // compileAndEmit). A canceled build must abort rather than run the project's + // compile to completion. + name: "build mode", + args: []string{"-b"}, + files: FileMap{ + "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "composite": true, "strict": true } }`, + "/home/src/workspaces/project/main.ts": badSource, + }, + }, } for _, tc := range testCases { diff --git a/internal/execute/watcher.go b/internal/execute/watcher.go index 07e8f096ed5..8bb2907e5c5 100644 --- a/internal/execute/watcher.go +++ b/internal/execute/watcher.go @@ -527,8 +527,15 @@ func (w *Watcher) evictChangedSourceFiles(changedPaths map[string]fswatch.EventK } func (w *Watcher) compileAndEmit() tsc.CompileAndEmitResult { - // Watch-cycle cancellation is handled at the RunLoop level (see WatchManager.RunLoop), - // so the per-cycle compile runs to completion. + // Watch-cycle cancellation is currently handled only at the RunLoop level (see + // WatchManager.RunLoop), which checks ctx.Done() between cycles; a compile already + // in flight runs to completion. + // + // TODO: thread the RunLoop context through DoCycle -> doBuild -> compileAndEmit and + // pass it here instead of context.Background(), so a long rebuild becomes + // interruptible at the checker's granularity. Doing so also requires handling an + // ExitStatusCanceled result (discard the partial cycle, keep the prior program) + // rather than reporting incomplete diagnostics. return tsc.EmitFilesAndReportErrors(context.Background(), tsc.EmitInput{ Sys: w.sys, ProgramLike: w.program, From 17169ec82bb4645d79a2ad25a8dd55a6f41fdaf3 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 10 Jul 2026 13:42:36 -0700 Subject: [PATCH 03/14] Tighten cancellation comments Trim the comments added on this branch for succinctness: keep the "no error channel" rationale once in forEachCheckerGroupDo and cross-reference it from the other guards, drop restated/tutorial prose. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/compiler/checkerpool.go | 18 +++------- internal/compiler/program.go | 12 +++---- internal/execute/build/buildtask.go | 8 ++--- internal/execute/build/orchestrator.go | 20 +++++------ internal/execute/tsc/emit.go | 5 ++- internal/execute/tsctests/tsccancel_test.go | 40 +++++++-------------- internal/execute/watcher.go | 13 +++---- 7 files changed, 40 insertions(+), 76 deletions(-) diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index be66fe1ac8f..ac8d80ce4c8 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -156,20 +156,10 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So defer p.locks[checkerIdx].Unlock() for i, file := range files { checker := p.checkers[checkerIdx] - // Stop feeding this checker once cancellation is in play: ctx.Err() - // means nothing we produce will be used, and WasCanceled() means the - // checker is already poisoned so reusing it would panic in - // checkNotCanceled. The two coincide in the compile path today (one - // context, single-use pool), but WasCanceled() states the actual reuse - // precondition rather than a proxy for it, and guards against a checker - // canceled by some other context. - // - // This out-of-band check is only necessary because the diagnostics APIs - // return a bare []*ast.Diagnostic with no error channel: a canceled check - // yields an empty (incomplete) slice indistinguishable from a clean result, - // so cancellation is signaled via checker state rather than a returned error. - // If those APIs returned (diags, error), the caller would stop on the error - // and this guard would be unnecessary. + // A canceled checker panics on reuse (checkNotCanceled), so stop feeding + // it more files. This guard is needed because the diagnostics APIs return + // []*ast.Diagnostic with no error channel: a canceled check is signaled by + // checker state, not a returned error. if ctx.Err() != nil || checker.WasCanceled() { break } diff --git a/internal/compiler/program.go b/internal/compiler/program.go index 27563fd31fc..c99bea15b38 100644 --- a/internal/compiler/program.go +++ b/internal/compiler/program.go @@ -613,8 +613,8 @@ func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, source continue } wg.Queue(func() { - // A checker obtained from the pool may already be in a canceled state from a - // prior file; reusing it would panic in checkNotCanceled. Skip once canceled. + // Skip once canceled: a pooled checker may already be canceled from a + // prior file, and reusing it would panic in checkNotCanceled. if ctx.Err() != nil { return } @@ -1820,11 +1820,9 @@ func GetDiagnosticsOfAnyProgram( if len(allDiagnostics) == configFileParsingDiagnosticsLength { allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics) - // If checking was canceled, the checker is now in a canceled state and must not - // be reused (GetGlobalDiagnostics/GetDeclarationDiagnostics would panic in - // checkNotCanceled). The diagnostics gathered so far are incomplete and will be - // discarded by the caller, so stop here. See checkerPool.forEachCheckerGroupDo - // for why this out-of-band check is needed (diagnostics APIs have no error channel). + // Once canceled, the checker must not be reused (GetGlobalDiagnostics / + // GetDeclarationDiagnostics would panic in checkNotCanceled). The partial + // diagnostics are discarded by the caller. See forEachCheckerGroupDo. if ctx.Err() != nil { return allDiagnostics } diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 49a0fe45107..6d73a5c40ba 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -235,11 +235,9 @@ func (t *BuildTask) compileAndEmit(ctx context.Context, orchestrator *Orchestrat t.result.exitStatus = result.Status t.result.statistics = statistics if result.Status == tsc.ExitStatusCanceled { - // The compile was canceled (e.g. SIGINT). Its result is incomplete - // (result.EmitResult is nil), so do not update output timestamps or mark the - // project up-to-date, and leave buildKind as None so it is not counted as - // built. report() propagates the canceled exit status (it dominates via max), - // and rangeTask stops scheduling further projects. + // Canceled: the result is incomplete (EmitResult is nil). Don't update + // timestamps, mark the project up-to-date, or count it as built. report() + // still propagates the canceled status. return } t.packageJsons = t.result.program.PackageJsonLookupPaths() diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index 99aa9bceeef..e106f9cb7c3 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -230,8 +230,8 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { } o.GenerateGraph(nil) if o.opts.Command.CompilerOptions.Watch.IsTrue() { - // In watch mode the initial build, like each watch cycle, runs to completion; - // cancellation is observed at the RunLoop boundary (see the TODO in DoCycle). + // Watch mode: the initial build runs to completion; cancellation is observed + // at the RunLoop boundary (see the TODO in DoCycle). result := o.buildOrClean(context.Background()) o.Watch(ctx) result.Watcher = o @@ -569,10 +569,8 @@ func (o *Orchestrator) DoCycle() { o.GenerateGraphReusingOldTasks() } - // TODO: like the CLI watcher, a build watch cycle runs to completion; cancellation - // is only observed between cycles in WatchManager.RunLoop. Thread the RunLoop context - // through DoCycle to make a long rebuild interruptible (and handle ExitStatusCanceled - // by discarding the partial cycle). + // TODO: thread the RunLoop context through DoCycle so a long rebuild is + // interruptible mid-cycle, not just between cycles (see the CLI watcher's TODO). o.buildOrClean(context.Background()) o.updateWatch() desiredDirs := o.computeDesiredWatches() @@ -600,9 +598,8 @@ func (o *Orchestrator) buildOrClean(ctx context.Context) tsc.CommandLineResult { o.buildOrCleanProject(ctx, task, path, &buildResult) }) if ctx.Err() != nil { - // The build was canceled (e.g. SIGINT). rangeTask stops scheduling further - // projects; report the aborted status even if cancellation landed before any - // project produced one. + // Report the aborted status even if cancellation landed before any project + // produced one. buildResult.result.Status = tsc.ExitStatusCanceled } } else { @@ -639,9 +636,8 @@ func (o *Orchestrator) rangeTask(ctx context.Context, f func(ctx context.Context } runTask := func() { for path, task, ok := getNextTask(); ok; path, task, ok = getNextTask() { - // Stop scheduling further projects once canceled (e.g. SIGINT). Tasks - // already in flight finish on their own; the compile they run is - // interruptible via the context threaded into compileAndEmit. + // Stop scheduling further projects once canceled; in-flight tasks finish + // (their compile is interruptible via ctx). if ctx.Err() != nil { return } diff --git a/internal/execute/tsc/emit.go b/internal/execute/tsc/emit.go index 6a8f2d1de60..33dbeca6437 100644 --- a/internal/execute/tsc/emit.go +++ b/internal/execute/tsc/emit.go @@ -111,9 +111,8 @@ func EmitFilesAndReportErrors(ctx context.Context, input EmitInput) (result Comp }, ) - // If the compile was canceled (e.g. SIGINT), the checker stops early and the - // diagnostics above are incomplete. Do not emit or report them as a complete - // result; abort with a distinct status instead. + // On cancellation the diagnostics above are incomplete; abort rather than emit + // or report them as a complete result. if ctx.Err() != nil { result.Status = ExitStatusCanceled return result diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index c31e606a82f..27a2411fa43 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -9,20 +9,15 @@ import ( "github.com/microsoft/typescript-go/internal/execute/tsc" ) -// TestTscNoEmitCancellation verifies that interrupting a `--noEmit` compile via -// the context passed to execute.CommandLine (which the CLI wires to SIGINT/SIGTERM -// in cmd/tsgo/main.go) aborts the compile promptly rather than running it to -// completion. -// -// A pre-canceled context deterministically exercises the same isCanceled() polling -// the checker uses for a mid-flight SIGINT (see internal/checker/utilities.go), so -// it covers both the "already canceled" and "canceled during check" cases. +// TestTscNoEmitCancellation verifies that a canceled context (wired to SIGINT in +// cmd/tsgo/main.go) aborts a compile instead of running it to completion. A +// pre-canceled context deterministically hits the same checker cancellation polling +// a mid-flight SIGINT would. func TestTscNoEmitCancellation(t *testing.T) { t.Parallel() - // Each file contains a type error so we can prove whether the checker ran to - // completion: if it did, the diagnostic is reported; if the compile was - // abandoned on cancellation, it is not. + // A type error lets us tell whether the checker ran to completion: if it did, + // the diagnostic is reported; if aborted, it is not. const badSource = `const x: number = "not a number";` testCases := []struct { @@ -31,8 +26,7 @@ func TestTscNoEmitCancellation(t *testing.T) { files FileMap }{ { - // Single file: exercises the top-level cancellation short-circuit in - // EmitFilesAndReportErrors / GetDiagnosticsOfAnyProgram. + // Top-level short-circuit in EmitFilesAndReportErrors. name: "single file", args: []string{"--noEmit"}, files: FileMap{ @@ -41,11 +35,9 @@ func TestTscNoEmitCancellation(t *testing.T) { }, }, { - // Multiple files under --singleThreaded funnel through a single checker, - // so the per-file loop in checkerPool.forEachCheckerGroupDo reuses the - // same checker across files. Once the first file cancels it, reusing it - // for the next file would panic in checkNotCanceled without the guard - // there. This case pins that guard. + // --singleThreaded funnels all files through one checker, so + // forEachCheckerGroupDo reuses it across files. Pins the guard that stops + // reuse after cancellation (else checkNotCanceled panics on the 2nd file). name: "multi file single checker", args: []string{"--noEmit", "--singleThreaded"}, files: FileMap{ @@ -56,10 +48,7 @@ func TestTscNoEmitCancellation(t *testing.T) { }, }, { - // Build mode (`tsc -b`): exercises the context threaded through the build - // orchestrator (Start -> buildOrClean -> rangeTask -> buildProject -> - // compileAndEmit). A canceled build must abort rather than run the project's - // compile to completion. + // `tsc -b`: exercises the context threaded through the build orchestrator. name: "build mode", args: []string{"-b"}, files: FileMap{ @@ -82,15 +71,12 @@ func TestTscNoEmitCancellation(t *testing.T) { result := execute.CommandLine(ctx, sys, tc.args, sys) - // The compile should short-circuit with a distinct canceled status - // instead of running the checker to completion (and it must not panic - // reusing a canceled checker). + // Aborts with a distinct status (and must not panic reusing a canceled checker). if result.Status != tsc.ExitStatusCanceled { t.Errorf("status = %v, want ExitStatusCanceled (compile should abort on cancellation)", result.Status) } - // Because the check was abandoned, its (incomplete) diagnostics must not - // be reported: the type errors should be absent from the output. + // Aborted checks must not report their incomplete diagnostics. if out := sys.getOutput(true); strings.Contains(out, "error TS") { t.Errorf("expected no diagnostics to be reported after cancellation; got output:\n%s", out) } diff --git a/internal/execute/watcher.go b/internal/execute/watcher.go index 8bb2907e5c5..1cc9f0ef26f 100644 --- a/internal/execute/watcher.go +++ b/internal/execute/watcher.go @@ -527,15 +527,12 @@ func (w *Watcher) evictChangedSourceFiles(changedPaths map[string]fswatch.EventK } func (w *Watcher) compileAndEmit() tsc.CompileAndEmitResult { - // Watch-cycle cancellation is currently handled only at the RunLoop level (see - // WatchManager.RunLoop), which checks ctx.Done() between cycles; a compile already - // in flight runs to completion. + // Watch cancellation is only observed between cycles (WatchManager.RunLoop); a + // compile in flight runs to completion. // - // TODO: thread the RunLoop context through DoCycle -> doBuild -> compileAndEmit and - // pass it here instead of context.Background(), so a long rebuild becomes - // interruptible at the checker's granularity. Doing so also requires handling an - // ExitStatusCanceled result (discard the partial cycle, keep the prior program) - // rather than reporting incomplete diagnostics. + // TODO: thread the RunLoop context through DoCycle -> doBuild -> compileAndEmit so + // a long rebuild is interruptible mid-cycle. That also means handling an + // ExitStatusCanceled result (discard the partial cycle, keep the prior program). return tsc.EmitFilesAndReportErrors(context.Background(), tsc.EmitInput{ Sys: w.sys, ProgramLike: w.program, From 1deebfda83813bd4a839321fcf6749bc99903011 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 10 Jul 2026 16:46:11 -0700 Subject: [PATCH 04/14] Fix build cancellation deadlock and partial-state reporting The rangeTask worker loop returned early on cancellation without running the task's f, so its done/reportDone channels never closed and any in-flight task waiting on them (waitOnUpstream / the report chain) could deadlock. Instead always run f for every fetched task and observe cancellation inside buildProject: skip the compile but still complete the task lifecycle. Make waitOnUpstream and updateDownstream ctx-aware so waiters escape on cancel and partial state is never propagated downstream. Drop the buildOrClean status override; a canceled task surfaces ExitStatusCanceled through its own report(). Honor cancellation during a watch-mode initial build too, but report success on that path: Ctrl-C is the expected way to exit a watch, so a non-zero code would make normal shutdown look like a failure. One-shot tsc -b still returns ExitStatusCanceled. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/execute/build/buildtask.go | 39 ++++++++++++++----- internal/execute/build/orchestrator.go | 28 ++++++------- .../execute/tsctests/watcher_race_test.go | 5 ++- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 6d73a5c40ba..f67ef42ed0c 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -74,9 +74,15 @@ type BuildTask struct { dirty bool } -func (t *BuildTask) waitOnUpstream() { +func (t *BuildTask) waitOnUpstream(ctx context.Context) { for _, upstream := range t.upStream { - <-upstream.task.done + select { + case <-upstream.task.done: + case <-ctx.Done(): + // Canceled while waiting: stop blocking. buildProject observes the + // cancellation and completes this task without building. + return + } } } @@ -123,13 +129,19 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { // Wait on upstream tasks to complete - t.waitOnUpstream() - if t.pending.Load() { + t.waitOnUpstream(ctx) + // Canceled before we started the compile: skip the expensive work but still fall + // through to unblockDownstream so downstream and report waiters don't deadlock. An + // in-flight compile aborts on its own (the checker polls ctx) and surfaces + // ExitStatusCanceled via compileAndEmit. + if ctx.Err() != nil { + t.result.exitStatus = tsc.ExitStatusCanceled + } else if t.pending.Load() { t.status = t.getUpToDateStatus(orchestrator, path) t.reportUpToDateStatus(orchestrator) if !t.handleStatusThatDoesntRequireBuild(orchestrator) { t.compileAndEmit(ctx, orchestrator, path) - t.updateDownstream(orchestrator, path) + t.updateDownstream(ctx, orchestrator, path) } else { if t.resolved != nil { for _, diagnostic := range t.resolved.GetConfigFileParsingDiagnostics() { @@ -152,7 +164,13 @@ func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator t.unblockDownstream() } -func (t *BuildTask) updateDownstream(orchestrator *Orchestrator, path tspath.Path) { +func (t *BuildTask) updateDownstream(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { + // A canceled compile leaves t.status and t.result partial (no buildKind, stale + // up-to-date status). Don't propagate that to downstream tasks: on cancellation + // buildProject reports ExitStatusCanceled and nothing else. + if ctx.Err() != nil { + return + } if t.isInitialCycle { return } @@ -235,9 +253,12 @@ func (t *BuildTask) compileAndEmit(ctx context.Context, orchestrator *Orchestrat t.result.exitStatus = result.Status t.result.statistics = statistics if result.Status == tsc.ExitStatusCanceled { - // Canceled: the result is incomplete (EmitResult is nil). Don't update - // timestamps, mark the project up-to-date, or count it as built. report() - // still propagates the canceled status. + // Canceled: the result is incomplete (EmitResult is nil). Leave the task + // partial on purpose -- don't update timestamps, set buildKind, record + // packageJsons, or overwrite t.status -- so nothing partial is reported. The + // caller (buildProject) skips updateDownstream, and report() sees buildKind + // unset, so the project is not counted as built. Only ExitStatusCanceled + // propagates. return } t.packageJsons = t.result.program.PackageJsonLookupPaths() diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index e106f9cb7c3..e3373b72ff5 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -229,16 +229,20 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { o.watchStatusReporter(ast.NewCompilerDiagnostic(diagnostics.Starting_compilation_in_watch_mode)) } o.GenerateGraph(nil) + result := o.buildOrClean(ctx) if o.opts.Command.CompilerOptions.Watch.IsTrue() { - // Watch mode: the initial build runs to completion; cancellation is observed - // at the RunLoop boundary (see the TODO in DoCycle). - result := o.buildOrClean(context.Background()) + // If we were already cancelled return now, but treat it as success. + // in watch mode this is the only way to exit. + if result.Status == tsc.ExitStatusCanceled { + result.Status = tsc.ExitStatusSuccess + return result + } o.Watch(ctx) result.Watcher = o return result } // Non-watch `tsc -b`: honor cancellation so a long build responds to SIGINT. - return o.buildOrClean(ctx) + return result } func (o *Orchestrator) Watch(ctx context.Context) { @@ -597,11 +601,8 @@ func (o *Orchestrator) buildOrClean(ctx context.Context) tsc.CommandLineResult { o.rangeTask(ctx, func(ctx context.Context, path tspath.Path, task *BuildTask) { o.buildOrCleanProject(ctx, task, path, &buildResult) }) - if ctx.Err() != nil { - // Report the aborted status even if cancellation landed before any project - // produced one. - buildResult.result.Status = tsc.ExitStatusCanceled - } + // A canceled task surfaces ExitStatusCanceled through its own report(), so the + // aggregated status reflects cancellation without a separate override here. } else { // Circularity errors prevent any project from being built buildResult.result.Status = tsc.ExitStatusProjectReferenceCycle_OutputsSkipped @@ -636,11 +637,10 @@ func (o *Orchestrator) rangeTask(ctx context.Context, f func(ctx context.Context } runTask := func() { for path, task, ok := getNextTask(); ok; path, task, ok = getNextTask() { - // Stop scheduling further projects once canceled; in-flight tasks finish - // (their compile is interruptible via ctx). - if ctx.Err() != nil { - return - } + // f is called for every task, even after cancellation: each task must + // complete its lifecycle (close its done/reportDone channels) or the + // upstream/report waiters of other tasks deadlock. Cancellation is observed + // inside buildProject, which skips the compile but still completes the task. f(ctx, path, task) } } diff --git a/internal/execute/tsctests/watcher_race_test.go b/internal/execute/tsctests/watcher_race_test.go index fbce8019218..280c298c1fe 100644 --- a/internal/execute/tsctests/watcher_race_test.go +++ b/internal/execute/tsctests/watcher_race_test.go @@ -286,8 +286,11 @@ func TestBuildWatchStopsWhenContextIsCancelled(t *testing.T) { select { case result := <-resultCh: + // Cancellation is honored during the initial build: it aborts promptly without + // establishing a watcher. But cancellation is the expected way to end a watch, + // so it still reports success -- Ctrl-C is not a build failure. assert.Equal(t, result.Status, tsc.ExitStatusSuccess) - assert.Assert(t, result.Watcher != nil) + assert.Assert(t, result.Watcher == nil) case <-time.After(2 * time.Second): t.Fatal("build watch did not stop after context cancellation") } From 2ba1544e457a3d85ded064aeac8193b314077171 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Fri, 10 Jul 2026 23:34:15 -0700 Subject: [PATCH 05/14] Fix cancellation panics/crashes and match tsc exit codes Addresses review feedback on the cancellation work: - EmitFilesAndReportErrors: re-check ctx after Emit. Emit returns nil when canceled partway through (e.g. the internal no-emit-on-error recheck), which previously fell through to Status=Success with a nil EmitResult and crashed in EmitAndReportStatistics. Guard EmitResult==nil there too. - checkerPool.GetGlobalDiagnostics: skip WasCanceled checkers. A checker canceled mid-check panics in checkNotCanceled when asked for global diagnostics; this fired from emitBuildInfo -> ensureHasErrorsForState, a path the earlier call-site guards missed. Fixing it in the pool covers the class, so the now-redundant guard in GetDiagnosticsOfAnyProgram is removed. - Drop the speculative ctx guard in collectCheckerDiagnosticsFromFiles: it only ran on the project-system pool path, which tsc never exercises. - cmd/tsgo: capture the interrupting signal (signal.Notify, not NotifyContext, whose context can't reveal the signal) and re-raise it on cancellation so the process exits with the conventional code -- 130 for SIGINT, 143 for SIGTERM -- matching the JS tsc, instead of the internal ExitStatusCanceled (6). - Tests: add TestTscMidCheckCancellation, which cancels *during* checking (the pre-canceled test cannot) and pins both the pool skip and the forEachCheckerGroupDo guard against checkNotCanceled panics. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/tsgo/main.go | 35 ++++- internal/compiler/checkerpool.go | 12 +- internal/compiler/program.go | 12 +- internal/execute/build/buildtask.go | 6 +- internal/execute/build/orchestrator.go | 3 +- internal/execute/tsc/emit.go | 11 +- internal/execute/tsctests/tsccancel_test.go | 137 ++++++++++++++++++-- internal/execute/watcher.go | 7 +- 8 files changed, 182 insertions(+), 41 deletions(-) diff --git a/cmd/tsgo/main.go b/cmd/tsgo/main.go index 8d6816fa3d4..7d76dadd9af 100644 --- a/cmd/tsgo/main.go +++ b/cmd/tsgo/main.go @@ -8,6 +8,7 @@ import ( "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/execute" + "github.com/microsoft/typescript-go/internal/execute/tsc" ) func main() { @@ -25,8 +26,38 @@ func runMain() int { return runAPI(args[1:]) } } - ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) - defer stop() + + // Notify on our own channel rather than using signal.NotifyContext: we need the + // actual signal so a canceled run can exit with the conventional 128+signum code, + // matching the JS tsc (which installs no handler and lets node's default fire). + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) + defer signal.Stop(sigCh) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var receivedSignal os.Signal + go func() { + select { + case receivedSignal = <-sigCh: + cancel() + case <-ctx.Done(): + } + }() + result := execute.CommandLine(ctx, newSystem(), args, nil) + + if result.Status == tsc.ExitStatusCanceled && receivedSignal != nil { + // A signal interrupted the run. Restore the default disposition and re-raise so + // the process terminates via the signal itself: this yields the conventional + // exit code (130 for SIGINT, 143 for SIGTERM) and lets the runtime reset the + // terminal, exactly as an unhandled signal would. + signal.Reset(receivedSignal) + if sig, ok := receivedSignal.(syscall.Signal); ok { + _ = syscall.Kill(os.Getpid(), sig) + // Block until the re-raised signal is delivered; do not fall through to a + // normal return, which would exit 6 and mask the signal. + select {} + } + } return int(result.Status) } diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index ac8d80ce4c8..8f0223e30a9 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -137,6 +137,11 @@ func (p *checkerPool) GetGlobalDiagnostics() []*ast.Diagnostic { p.createCheckers() globalDiagnostics := make([][]*ast.Diagnostic, len(p.checkers)) p.forEachCheckerParallel(func(idx int, checker *checker.Checker) { + // A canceled checker panics if asked for diagnostics (checkNotCanceled), and + // its results are discarded once canceled anyway. Skip it. + if checker.WasCanceled() { + return + } globalDiagnostics[idx] = checker.GetGlobalDiagnostics() }) return SortAndDeduplicateDiagnostics(slices.Concat(globalDiagnostics...)) @@ -156,11 +161,8 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So defer p.locks[checkerIdx].Unlock() for i, file := range files { checker := p.checkers[checkerIdx] - // A canceled checker panics on reuse (checkNotCanceled), so stop feeding - // it more files. This guard is needed because the diagnostics APIs return - // []*ast.Diagnostic with no error channel: a canceled check is signaled by - // checker state, not a returned error. - if ctx.Err() != nil || checker.WasCanceled() { + // Check for cancellation + if ctx.Err() != nil { break } if checker == p.fileAssociations[file] { diff --git a/internal/compiler/program.go b/internal/compiler/program.go index c99bea15b38..5ffb98a071d 100644 --- a/internal/compiler/program.go +++ b/internal/compiler/program.go @@ -613,11 +613,6 @@ func (p *Program) collectCheckerDiagnosticsFromFiles(ctx context.Context, source continue } wg.Queue(func() { - // Skip once canceled: a pooled checker may already be canceled from a - // prior file, and reusing it would panic in checkNotCanceled. - if ctx.Err() != nil { - return - } c, done := p.checkerPool.GetChecker(ctx, file) diagnostics[i] = collect(ctx, c, file) done() @@ -1820,13 +1815,8 @@ func GetDiagnosticsOfAnyProgram( if len(allDiagnostics) == configFileParsingDiagnosticsLength { allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics) - // Once canceled, the checker must not be reused (GetGlobalDiagnostics / - // GetDeclarationDiagnostics would panic in checkNotCanceled). The partial - // diagnostics are discarded by the caller. See forEachCheckerGroupDo. - if ctx.Err() != nil { - return allDiagnostics - } // Ask for the global diagnostics again (they were empty above); we may have found new during checking, e.g. missing globals. + // Safe after cancellation: GetGlobalDiagnostics skips canceled checkers. allDiagnostics = append(allDiagnostics, program.GetGlobalDiagnostics(ctx)...) } diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index f67ef42ed0c..58138ea0d83 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -254,11 +254,7 @@ func (t *BuildTask) compileAndEmit(ctx context.Context, orchestrator *Orchestrat t.result.statistics = statistics if result.Status == tsc.ExitStatusCanceled { // Canceled: the result is incomplete (EmitResult is nil). Leave the task - // partial on purpose -- don't update timestamps, set buildKind, record - // packageJsons, or overwrite t.status -- so nothing partial is reported. The - // caller (buildProject) skips updateDownstream, and report() sees buildKind - // unset, so the project is not counted as built. Only ExitStatusCanceled - // propagates. + // partial on purpose. Only ExitStatusCanceled propagates. return } t.packageJsons = t.result.program.PackageJsonLookupPaths() diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index e3373b72ff5..e68a6f92a7a 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -573,8 +573,7 @@ func (o *Orchestrator) DoCycle() { o.GenerateGraphReusingOldTasks() } - // TODO: thread the RunLoop context through DoCycle so a long rebuild is - // interruptible mid-cycle, not just between cycles (see the CLI watcher's TODO). + // TODO: propagate a proper context here and support cancellation with cycle o.buildOrClean(context.Background()) o.updateWatch() desiredDirs := o.computeDesiredWatches() diff --git a/internal/execute/tsc/emit.go b/internal/execute/tsc/emit.go index 33dbeca6437..c6fcb3ed471 100644 --- a/internal/execute/tsc/emit.go +++ b/internal/execute/tsc/emit.go @@ -46,8 +46,8 @@ type EmitInput struct { func EmitAndReportStatistics(ctx context.Context, input EmitInput) (CompileAndEmitResult, *Statistics) { var statistics *Statistics result := EmitFilesAndReportErrors(ctx, input) - if result.Status != ExitStatusSuccess { - // compile exited early + if result.Status != ExitStatusSuccess || result.EmitResult == nil { + // compile exited early (e.g. errors or cancellation); EmitResult may be nil return result, nil } result.times.totalTime = input.Sys.SinceStart() @@ -125,6 +125,13 @@ func EmitFilesAndReportErrors(ctx context.Context, input EmitInput) (result Comp WriteFile: input.WriteFile, }) result.times.emitTime += input.Sys.Now().Sub(emitStart) + // Emit returns nil if it was canceled partway through (e.g. cancellation + // during the internal no-emit-on-error recheck). Abort rather than report a + // nil result as success. + if ctx.Err() != nil { + result.Status = ExitStatusCanceled + return result + } } if emitResult != nil { allDiagnostics = append(allDiagnostics, emitResult.Diagnostics...) diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index 27a2411fa43..fcaa9747357 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -3,17 +3,18 @@ package tsctests import ( "context" "strings" + "sync/atomic" "testing" + "time" "github.com/microsoft/typescript-go/internal/execute" "github.com/microsoft/typescript-go/internal/execute/tsc" ) -// TestTscNoEmitCancellation verifies that a canceled context (wired to SIGINT in -// cmd/tsgo/main.go) aborts a compile instead of running it to completion. A -// pre-canceled context deterministically hits the same checker cancellation polling -// a mid-flight SIGINT would. -func TestTscNoEmitCancellation(t *testing.T) { +// TestTscPreCanceledCompilation verifies that a context canceled before the compile +// starts aborts immediately with ExitStatusCanceled and reports no diagnostics, +// without panicking on checker reuse. +func TestTscPreCanceledCompilation(t *testing.T) { t.Parallel() // A type error lets us tell whether the checker ran to completion: if it did, @@ -36,8 +37,7 @@ func TestTscNoEmitCancellation(t *testing.T) { }, { // --singleThreaded funnels all files through one checker, so - // forEachCheckerGroupDo reuses it across files. Pins the guard that stops - // reuse after cancellation (else checkNotCanceled panics on the 2nd file). + // forEachCheckerGroupDo reuses it across files. name: "multi file single checker", args: []string{"--noEmit", "--singleThreaded"}, files: FileMap{ @@ -67,7 +67,7 @@ func TestTscNoEmitCancellation(t *testing.T) { }, false) ctx, cancel := context.WithCancel(context.Background()) - cancel() // simulate SIGINT delivered before/at the start of the compile + cancel() // simulate SIGINT delivered before the compile starts result := execute.CommandLine(ctx, sys, tc.args, sys) @@ -83,3 +83,124 @@ func TestTscNoEmitCancellation(t *testing.T) { }) } } + +// cancelAfterNPolls is a context that reports itself canceled only after Err has +// been polled pollThreshold times while still uncanceled. The checker polls +// ctx.Err() between top-level statements (checkSourceElements), so this lands the +// cancellation *after* checking has begun rather than before it starts -- the case +// a pre-canceled context cannot exercise. Once tripped it stays canceled. +type cancelAfterNPolls struct { + context.Context + pollThreshold int32 + polls atomic.Int32 + tripped atomic.Bool + done chan struct{} +} + +func newCancelAfterNPolls(pollThreshold int32) *cancelAfterNPolls { + return &cancelAfterNPolls{ + Context: context.Background(), + pollThreshold: pollThreshold, + done: make(chan struct{}), + } +} + +func (c *cancelAfterNPolls) Err() error { + if c.tripped.Load() { + return context.Canceled + } + if c.polls.Add(1) > c.pollThreshold { + if c.tripped.CompareAndSwap(false, true) { + close(c.done) + } + return context.Canceled + } + return nil +} + +func (c *cancelAfterNPolls) Done() <-chan struct{} { + return c.done +} + +// TestTscMidCheckCancellation cancels the context after type-checking has already +// begun (not before it starts). This exercises the paths a pre-canceled context +// skips: a checker actually runs, sets wasCanceled, and would panic in +// checkNotCanceled on the second GetGlobalDiagnostics / on reuse across files if +// the cancellation guards were missing. +func TestTscMidCheckCancellation(t *testing.T) { + t.Parallel() + + // Enough top-level statements across multiple files that the checker polls + // ctx.Err() many times, so cancellation reliably lands mid-check. + var manyStatements strings.Builder + for i := range 50 { + manyStatements.WriteString("export const v") + manyStatements.WriteString(strings.Repeat("x", i+1)) + manyStatements.WriteString(`: number = "not a number";` + "\n") + } + src := manyStatements.String() + + testCases := []struct { + name string + args []string + }{ + { + // Single checker reused across files: after it cancels on an early file, + // forEachCheckerGroupDo must stop feeding it later files. + name: "single checker", + args: []string{"--noEmit", "--singleThreaded"}, + }, + { + // tsc -b through the incremental program + orchestrator. + name: "build mode", + args: []string{"-b"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + files := FileMap{ + "/home/src/workspaces/project/a.ts": src, + "/home/src/workspaces/project/b.ts": src, + "/home/src/workspaces/project/c.ts": src, + } + if tc.name == "build mode" { + files["/home/src/workspaces/project/tsconfig.json"] = `{ "compilerOptions": { "composite": true, "strict": true } }` + } else { + files["/home/src/workspaces/project/tsconfig.json"] = `{ "compilerOptions": { "noEmit": true, "strict": true } }` + } + + sys := newTestSys(&tscInput{ + commandLineArgs: tc.args, + files: files, + }, false) + + // Let checking start, then cancel. The threshold is small relative to the + // number of statements so cancellation lands well before checking finishes. + ctx := newCancelAfterNPolls(5) + + resultCh := make(chan tsc.CommandLineResult, 1) + go func() { + resultCh <- execute.CommandLine(ctx, sys, tc.args, sys) + }() + + select { + case result := <-resultCh: + // The run must abort (not run to completion) once canceled mid-check, + // and must not panic in checkNotCanceled. + if result.Status != tsc.ExitStatusCanceled { + t.Errorf("status = %v, want ExitStatusCanceled", result.Status) + } + // The cancellation must actually have landed mid-check, otherwise this + // test is not exercising what it claims. + if !ctx.tripped.Load() { + t.Error("expected cancellation to trip during checking, but it never did") + } + case <-time.After(30 * time.Second): + t.Fatal("compile did not abort after mid-check cancellation") + } + }) + } +} diff --git a/internal/execute/watcher.go b/internal/execute/watcher.go index 1cc9f0ef26f..fa92ed7cf33 100644 --- a/internal/execute/watcher.go +++ b/internal/execute/watcher.go @@ -527,12 +527,7 @@ func (w *Watcher) evictChangedSourceFiles(changedPaths map[string]fswatch.EventK } func (w *Watcher) compileAndEmit() tsc.CompileAndEmitResult { - // Watch cancellation is only observed between cycles (WatchManager.RunLoop); a - // compile in flight runs to completion. - // - // TODO: thread the RunLoop context through DoCycle -> doBuild -> compileAndEmit so - // a long rebuild is interruptible mid-cycle. That also means handling an - // ExitStatusCanceled result (discard the partial cycle, keep the prior program). + // TODO: propagate a proper context here to better support cancelation return tsc.EmitFilesAndReportErrors(context.Background(), tsc.EmitInput{ Sys: w.sys, ProgramLike: w.program, From af1a32af361cde69dfc8fd3a4d8233887839d860 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sat, 11 Jul 2026 00:45:09 -0700 Subject: [PATCH 06/14] Harden cancellation: fix emit re-entry panic, add sweep coverage Follow-up hardening after finding the call-site guards were incomplete: - HandleNoEmitOnError: bail when already canceled. During incremental emit it is re-invoked per affected file and re-runs GetDiagnosticsOfAnyProgram; on a canceled checker that panics in checkNotCanceled (getSemanticDiagnostics). This vector was not covered by the earlier guards. - Restore the ctx guard in GetDiagnosticsOfAnyProgram after getSemanticDiagnostics. It is independently needed for the --declaration path (no noEmitOnError, so HandleNoEmitOnError returns early): without it the subsequent GetDeclarationDiagnostics serializes types on a canceled checker and panics. - main.go: harden the signal re-raise. Capture the signal on a dedicated channel (no shared variable read across the goroutine) and, after re-raising, return 128+signum instead of blocking forever, so a failed Kill can't hang the process. - Tighten the checkerpool cancellation comment to say why it breaks. Tests (all with verified teeth -- each fix was reverted to confirm the test fails): - compiler: TestGetGlobalDiagnosticsAfterCancellation directly pins the pool's WasCanceled skip. - tsctests: TestTscMidCheckCancellation gains a declaration-emit case; new TestTscCancellationSweep cancels at every successive poll across the compile and into emit (incremental+noEmitOnError and declaration configs), asserting no panic and a consistent terminal status. This sweep is what surfaced the emit re-entry panic above. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/tsgo/main.go | 36 ++--- internal/compiler/checkerpool.go | 6 +- internal/compiler/checkerpool_test.go | 96 +++++++++++++ internal/compiler/program.go | 12 +- internal/execute/tsctests/tsccancel_test.go | 141 ++++++++++++++++---- 5 files changed, 245 insertions(+), 46 deletions(-) create mode 100644 internal/compiler/checkerpool_test.go diff --git a/cmd/tsgo/main.go b/cmd/tsgo/main.go index 7d76dadd9af..d5c510f3141 100644 --- a/cmd/tsgo/main.go +++ b/cmd/tsgo/main.go @@ -27,18 +27,21 @@ func runMain() int { } } - // Notify on our own channel rather than using signal.NotifyContext: we need the - // actual signal so a canceled run can exit with the conventional 128+signum code, - // matching the JS tsc (which installs no handler and lets node's default fire). + // Use signal.Notify with our own channel rather than signal.NotifyContext: the + // latter's context can't tell us which signal fired, and we need it to exit like + // the JS tsc, which installs no handler and lets node terminate via the signal. sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) defer signal.Stop(sigCh) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - var receivedSignal os.Signal + // canceledBy carries the interrupting signal (if any) to the code below. It is + // written before cancel() and thus before CommandLine can observe cancellation. + canceledBy := make(chan os.Signal, 1) go func() { select { - case receivedSignal = <-sigCh: + case sig := <-sigCh: + canceledBy <- sig cancel() case <-ctx.Done(): } @@ -46,17 +49,18 @@ func runMain() int { result := execute.CommandLine(ctx, newSystem(), args, nil) - if result.Status == tsc.ExitStatusCanceled && receivedSignal != nil { - // A signal interrupted the run. Restore the default disposition and re-raise so - // the process terminates via the signal itself: this yields the conventional - // exit code (130 for SIGINT, 143 for SIGTERM) and lets the runtime reset the - // terminal, exactly as an unhandled signal would. - signal.Reset(receivedSignal) - if sig, ok := receivedSignal.(syscall.Signal); ok { - _ = syscall.Kill(os.Getpid(), sig) - // Block until the re-raised signal is delivered; do not fall through to a - // normal return, which would exit 6 and mask the signal. - select {} + if result.Status == tsc.ExitStatusCanceled { + // A signal canceled the run. Re-raise it so we terminate via the signal itself, + // yielding the conventional exit code (130 for SIGINT, 143 for SIGTERM) and the + // terminal reset an unhandled signal would produce. + select { + case sig := <-canceledBy: + if s, ok := sig.(syscall.Signal); ok { + signal.Reset(s) + _ = syscall.Kill(os.Getpid(), s) + return 128 + int(s) // fallback in case the signal doesn't land promptly + } + default: } } return int(result.Status) diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index 8f0223e30a9..13e1ac05fae 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -160,12 +160,12 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So p.locks[checkerIdx].Lock() defer p.locks[checkerIdx].Unlock() for i, file := range files { - checker := p.checkers[checkerIdx] - // Check for cancellation + // Stop once canceled: feeding another file to a checker that already + // canceled mid-check would panic in checkNotCanceled. if ctx.Err() != nil { break } - if checker == p.fileAssociations[file] { + if checker := p.checkers[checkerIdx]; checker == p.fileAssociations[file] { cb(checker, i, file) } } diff --git a/internal/compiler/checkerpool_test.go b/internal/compiler/checkerpool_test.go new file mode 100644 index 00000000000..4a31da6f6d3 --- /dev/null +++ b/internal/compiler/checkerpool_test.go @@ -0,0 +1,96 @@ +package compiler_test + +import ( + "context" + "strings" + "sync/atomic" + "testing" + + "github.com/microsoft/typescript-go/internal/bundled" + "github.com/microsoft/typescript-go/internal/compiler" + "github.com/microsoft/typescript-go/internal/core" + "github.com/microsoft/typescript-go/internal/tsoptions" + "github.com/microsoft/typescript-go/internal/vfs/vfstest" +) + +// cancelAfterNPolls reports itself canceled only after Err has been polled +// pollThreshold times while still uncanceled, so cancellation lands after checking +// has begun rather than before it starts. Once tripped it stays canceled. +type cancelAfterNPolls struct { + context.Context + pollThreshold int32 + polls atomic.Int32 + tripped atomic.Bool + done chan struct{} +} + +func newCancelAfterNPolls(pollThreshold int32) *cancelAfterNPolls { + return &cancelAfterNPolls{Context: context.Background(), pollThreshold: pollThreshold, done: make(chan struct{})} +} + +func (c *cancelAfterNPolls) Err() error { + if c.tripped.Load() { + return context.Canceled + } + if c.polls.Add(1) > c.pollThreshold { + if c.tripped.CompareAndSwap(false, true) { + close(c.done) + } + return context.Canceled + } + return nil +} + +func (c *cancelAfterNPolls) Done() <-chan struct{} { return c.done } + +// TestGetGlobalDiagnosticsAfterCancellation pins the checker-pool behavior that a +// checker canceled mid-check is skipped by GetGlobalDiagnostics rather than reused +// (which panics in checkNotCanceled). This is the source-level guard that protects +// every GetGlobalDiagnostics caller, including emitBuildInfo's error-state probe, +// which has no surrounding cancellation check. +func TestGetGlobalDiagnosticsAfterCancellation(t *testing.T) { + t.Parallel() + + if !bundled.Embedded { + t.Skip("bundled files are not embedded") + } + + fs := bundled.WrapFS(vfstest.FromMap[any](nil, false /*useCaseSensitiveFileNames*/)) + + // Many statements with type errors across a few files: global diagnostics exist, + // and the checker polls often so cancellation lands mid-check. + var src strings.Builder + for i := range 50 { + src.WriteString("export const v") + src.WriteString(strings.Repeat("x", i+1)) + src.WriteString(`: number = "not a number";` + "\n") + } + for _, name := range []string{"/src/a.ts", "/src/b.ts", "/src/c.ts"} { + _ = fs.WriteFile(name, src.String()) + } + + // One checker so a single mid-check cancellation marks the checker the subsequent + // GetGlobalDiagnostics will visit. + oneChecker := 1 + program := compiler.NewProgram(compiler.ProgramOptions{ + Config: &tsoptions.ParsedCommandLine{ + ParsedConfig: &core.ParsedOptions{ + FileNames: []string{"/src/a.ts", "/src/b.ts", "/src/c.ts"}, + CompilerOptions: &core.CompilerOptions{Strict: core.TSTrue, Checkers: &oneChecker}, + }, + }, + Host: compiler.NewCompilerHost("/src", fs, bundled.LibPath(), nil, nil), + }) + + ctx := newCancelAfterNPolls(5) + + // Drive checking under the canceling context to cancel the checker. Discard the + // (partial) result; we only care that the checker is now canceled. + _ = program.GetSemanticDiagnostics(ctx, nil) + if !ctx.tripped.Load() { + t.Fatal("expected cancellation to trip during checking, but it never did") + } + + // The real assertion: this must not panic on the canceled checker. + _ = program.GetGlobalDiagnostics(ctx) +} diff --git a/internal/compiler/program.go b/internal/compiler/program.go index 5ffb98a071d..16d1858cb95 100644 --- a/internal/compiler/program.go +++ b/internal/compiler/program.go @@ -1761,6 +1761,11 @@ func HandleNoEmitOnError(ctx context.Context, program ProgramLike, files []*ast. if !program.Options().NoEmitOnError.IsTrue() { return nil // No emit on error is not set, so we can proceed with emitting } + if ctx.Err() != nil { + // Canceled: don't re-run diagnostics on checkers that may already be canceled + // (checkNotCanceled would panic). The emit is being abandoned regardless. + return nil + } diagnostics := GetDiagnosticsOfAnyProgram( ctx, @@ -1815,8 +1820,13 @@ func GetDiagnosticsOfAnyProgram( if len(allDiagnostics) == configFileParsingDiagnosticsLength { allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics) + // Stop once canceled: the diagnostics are discarded anyway, and the calls + // below (GetGlobalDiagnostics, GetDeclarationDiagnostics) reuse the now + // canceled checkers, which panics in checkNotCanceled. + if ctx.Err() != nil { + return allDiagnostics + } // Ask for the global diagnostics again (they were empty above); we may have found new during checking, e.g. missing globals. - // Safe after cancellation: GetGlobalDiagnostics skips canceled checkers. allDiagnostics = append(allDiagnostics, program.GetGlobalDiagnostics(ctx)...) } diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index fcaa9747357..e2f8ed4185b 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -130,30 +130,55 @@ func (c *cancelAfterNPolls) Done() <-chan struct{} { func TestTscMidCheckCancellation(t *testing.T) { t.Parallel() - // Enough top-level statements across multiple files that the checker polls - // ctx.Err() many times, so cancellation reliably lands mid-check. - var manyStatements strings.Builder + // Many top-level statements per file so the checker polls ctx.Err() often and + // cancellation reliably lands mid-check, across more than one file. + var badStatements strings.Builder for i := range 50 { - manyStatements.WriteString("export const v") - manyStatements.WriteString(strings.Repeat("x", i+1)) - manyStatements.WriteString(`: number = "not a number";` + "\n") + badStatements.WriteString("export const v") + badStatements.WriteString(strings.Repeat("x", i+1)) + badStatements.WriteString(`: number = "not a number";` + "\n") + } + badSrc := badStatements.String() + + // Inferred return types force the checker to serialize types during declaration + // emit (SerializeReturnTypeForSignature -> node reuse -> checkNotCanceled), a + // distinct reuse path from plain semantic checking. + var inferredSrc strings.Builder + for i := range 50 { + inferredSrc.WriteString("export function make") + inferredSrc.WriteString(strings.Repeat("x", i+1)) + inferredSrc.WriteString("() { return { a: 1, b: 'x', deep: [1, 2, 3] as const }; }\n") } - src := manyStatements.String() testCases := []struct { - name string - args []string + name string + args []string + tsconfig string + src string }{ { // Single checker reused across files: after it cancels on an early file, // forEachCheckerGroupDo must stop feeding it later files. - name: "single checker", - args: []string{"--noEmit", "--singleThreaded"}, + name: "single checker", + args: []string{"--noEmit", "--singleThreaded"}, + tsconfig: `{ "compilerOptions": { "noEmit": true, "strict": true } }`, + src: badSrc, }, { - // tsc -b through the incremental program + orchestrator. - name: "build mode", - args: []string{"-b"}, + // tsc -b through the incremental program + orchestrator, which also reaches + // GetGlobalDiagnostics from emitBuildInfo -> ensureHasErrorsForState. + name: "build mode", + args: []string{"-b"}, + tsconfig: `{ "compilerOptions": { "composite": true, "strict": true } }`, + src: badSrc, + }, + { + // Declaration emit reuses checkers to serialize types; a canceled checker + // must not be handed to GetDeclarationDiagnostics. + name: "declaration emit", + args: []string{"--noEmit", "--declaration", "--singleThreaded"}, + tsconfig: `{ "compilerOptions": { "noEmit": true, "declaration": true, "strict": true } }`, + src: inferredSrc.String(), }, } @@ -161,20 +186,14 @@ func TestTscMidCheckCancellation(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - files := FileMap{ - "/home/src/workspaces/project/a.ts": src, - "/home/src/workspaces/project/b.ts": src, - "/home/src/workspaces/project/c.ts": src, - } - if tc.name == "build mode" { - files["/home/src/workspaces/project/tsconfig.json"] = `{ "compilerOptions": { "composite": true, "strict": true } }` - } else { - files["/home/src/workspaces/project/tsconfig.json"] = `{ "compilerOptions": { "noEmit": true, "strict": true } }` - } - sys := newTestSys(&tscInput{ commandLineArgs: tc.args, - files: files, + files: FileMap{ + "/home/src/workspaces/project/tsconfig.json": tc.tsconfig, + "/home/src/workspaces/project/a.ts": tc.src, + "/home/src/workspaces/project/b.ts": tc.src, + "/home/src/workspaces/project/c.ts": tc.src, + }, }, false) // Let checking start, then cancel. The threshold is small relative to the @@ -204,3 +223,73 @@ func TestTscMidCheckCancellation(t *testing.T) { }) } } + +// TestTscCancellationSweep cancels at every successive point in the compile by +// increasing the poll threshold one step at a time. It walks cancellation past the +// check phase and into emit, covering windows a single fixed threshold would miss: +// the no-emit-on-error recheck that runs during emit, the emit-returns-nil path, and +// declaration-emit type serialization. At no threshold may the run panic, and it +// must always end Canceled or Success (if it finished before the trip). +func TestTscCancellationSweep(t *testing.T) { + t.Parallel() + + configs := []struct { + name string + tsconfig string + }{ + { + // noEmitOnError + incremental: emit performs the internal no-emit-on-error + // recheck, the path where a mid-emit cancellation makes Emit return nil. + name: "incremental noEmitOnError", + tsconfig: `{ "compilerOptions": { "outDir": "out", "incremental": true, "noEmitOnError": true, "strict": true } }`, + }, + { + // declaration emit serializes inferred types via the checker during emit. + name: "declaration", + tsconfig: `{ "compilerOptions": { "outDir": "out", "declaration": true, "noEmitOnError": true, "strict": true } }`, + }, + } + + for _, cfg := range configs { + t.Run(cfg.name, func(t *testing.T) { + t.Parallel() + files := FileMap{ + "/home/src/workspaces/project/tsconfig.json": cfg.tsconfig, + "/home/src/workspaces/project/a.ts": "export const a = 1;\nexport function f() { return { x: 1, y: 'z' }; }\n", + "/home/src/workspaces/project/b.ts": "export const b = 2;\nexport function g() { return [1, 2, 3] as const; }\n", + } + + // Upper bound comfortably exceeds a full clean run's poll count for this + // project, so the sweep covers check, the second global-diagnostics pass, + // and emit. + for threshold := int32(1); threshold <= 150; threshold++ { + sys := newTestSys(&tscInput{ + commandLineArgs: []string{"--singleThreaded"}, + files: files, + }, false) + ctx := newCancelAfterNPolls(threshold) + + var result tsc.CommandLineResult + func() { + defer func() { + if r := recover(); r != nil { + t.Fatalf("threshold=%d: panicked (want clean abort): %v", threshold, r) + } + }() + result = execute.CommandLine(ctx, sys, []string{"--singleThreaded"}, sys) + }() + + // Success only if cancellation never tripped (the build outran it); + // otherwise it must be a clean Canceled. Anything else means partial + // state leaked out. + if ctx.tripped.Load() { + if result.Status != tsc.ExitStatusCanceled { + t.Fatalf("threshold=%d: status = %v, want ExitStatusCanceled", threshold, result.Status) + } + } else if result.Status != tsc.ExitStatusSuccess { + t.Fatalf("threshold=%d: status = %v, want ExitStatusSuccess (cancellation never tripped)", threshold, result.Status) + } + } + }) + } +} From 14667dc1fb9ea6499633ce3cf939b83ac071b863 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sat, 11 Jul 2026 01:00:48 -0700 Subject: [PATCH 07/14] Correct updateDownstream cancellation comment The old comment claimed buildProject "reports ExitStatusCanceled and nothing else" when this guard fires. That is wrong at the completion boundary: if the compile finishes and cancellation trips exactly on this ctx.Err() poll, buildProject reports Success (the work completed) -- the guard just skips the now-pointless downstream propagation. Rewrite it to state what the guard actually protects: updateDownstream seeds dependents' in-memory rebuild state (never outputs), and must be skipped once canceled so a partially-emitted program's HasChangedDtsFile can't corrupt downstream up-to-date decisions. Note it is currently a no-op in one-shot -b (empty downStream) and becomes load-bearing once DoCycle threads a cancelable context into watch rebuilds. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/execute/build/buildtask.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 58138ea0d83..b39e940fb15 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -165,9 +165,14 @@ func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator } func (t *BuildTask) updateDownstream(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { - // A canceled compile leaves t.status and t.result partial (no buildKind, stale - // up-to-date status). Don't propagate that to downstream tasks: on cancellation - // buildProject reports ExitStatusCanceled and nothing else. + // This seeds dependents' in-memory rebuild state (status, pending) from this + // project's result; it never touches emitted outputs. Skip it once canceled: the + // result may be from a compile that aborted mid-emit, and propagating its partial + // HasChangedDtsFile would corrupt downstream up-to-date decisions. + // + // Today this only fires in one-shot `tsc -b`, where downStream is empty and the + // body below is a no-op anyway. It becomes load-bearing once DoCycle threads a + // real (cancelable) context into watch rebuilds, where downStream is populated. if ctx.Err() != nil { return } From e6bc6e1d88384f89675c10e3d3dcc793ed2b9093 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sat, 11 Jul 2026 01:23:05 -0700 Subject: [PATCH 08/14] Consolidate cancellation tests Merge the pre-canceled and mid-check tests into a single table-driven TestTscCancellationAborts: both assert the same thing (aborts with ExitStatusCanceled, no diagnostics leaked, no checkNotCanceled panic), differing only in when the signal fires. A per-case midCheck flag selects the cancellation strategy via a shared runWithCancellation helper, which also folds in the timeout guard. As a side benefit the mid-check cases now also assert no diagnostics are reported, which the old mid-check test didn't. TestTscCancellationSweep stays separate -- it exercises a distinct property (no panic / no partial-success across every cancellation point) with clean source. Teeth re-verified after the refactor: reverting each guard still fails the corresponding case. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/execute/tsctests/tsccancel_test.go | 239 ++++++++------------ 1 file changed, 100 insertions(+), 139 deletions(-) diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index e2f8ed4185b..ebb1839975a 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -11,84 +11,11 @@ import ( "github.com/microsoft/typescript-go/internal/execute/tsc" ) -// TestTscPreCanceledCompilation verifies that a context canceled before the compile -// starts aborts immediately with ExitStatusCanceled and reports no diagnostics, -// without panicking on checker reuse. -func TestTscPreCanceledCompilation(t *testing.T) { - t.Parallel() - - // A type error lets us tell whether the checker ran to completion: if it did, - // the diagnostic is reported; if aborted, it is not. - const badSource = `const x: number = "not a number";` - - testCases := []struct { - name string - args []string - files FileMap - }{ - { - // Top-level short-circuit in EmitFilesAndReportErrors. - name: "single file", - args: []string{"--noEmit"}, - files: FileMap{ - "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "noEmit": true, "strict": true } }`, - "/home/src/workspaces/project/main.ts": badSource, - }, - }, - { - // --singleThreaded funnels all files through one checker, so - // forEachCheckerGroupDo reuses it across files. - name: "multi file single checker", - args: []string{"--noEmit", "--singleThreaded"}, - files: FileMap{ - "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "noEmit": true, "strict": true } }`, - "/home/src/workspaces/project/a.ts": badSource, - "/home/src/workspaces/project/b.ts": badSource, - "/home/src/workspaces/project/c.ts": badSource, - }, - }, - { - // `tsc -b`: exercises the context threaded through the build orchestrator. - name: "build mode", - args: []string{"-b"}, - files: FileMap{ - "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "composite": true, "strict": true } }`, - "/home/src/workspaces/project/main.ts": badSource, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - sys := newTestSys(&tscInput{ - commandLineArgs: tc.args, - files: tc.files, - }, false) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() // simulate SIGINT delivered before the compile starts - - result := execute.CommandLine(ctx, sys, tc.args, sys) - - // Aborts with a distinct status (and must not panic reusing a canceled checker). - if result.Status != tsc.ExitStatusCanceled { - t.Errorf("status = %v, want ExitStatusCanceled (compile should abort on cancellation)", result.Status) - } - - // Aborted checks must not report their incomplete diagnostics. - if out := sys.getOutput(true); strings.Contains(out, "error TS") { - t.Errorf("expected no diagnostics to be reported after cancellation; got output:\n%s", out) - } - }) - } -} - // cancelAfterNPolls is a context that reports itself canceled only after Err has // been polled pollThreshold times while still uncanceled. The checker polls -// ctx.Err() between top-level statements (checkSourceElements), so this lands the -// cancellation *after* checking has begun rather than before it starts -- the case -// a pre-canceled context cannot exercise. Once tripped it stays canceled. +// ctx.Err() between top-level statements (checkSourceElements), so a small threshold +// lands the cancellation *after* checking has begun rather than before it starts -- +// the case a pre-canceled context cannot exercise. Once tripped it stays canceled. type cancelAfterNPolls struct { context.Context pollThreshold int32 @@ -118,74 +45,83 @@ func (c *cancelAfterNPolls) Err() error { return nil } -func (c *cancelAfterNPolls) Done() <-chan struct{} { - return c.done -} +func (c *cancelAfterNPolls) Done() <-chan struct{} { return c.done } -// TestTscMidCheckCancellation cancels the context after type-checking has already -// begun (not before it starts). This exercises the paths a pre-canceled context -// skips: a checker actually runs, sets wasCanceled, and would panic in -// checkNotCanceled on the second GetGlobalDiagnostics / on reuse across files if -// the cancellation guards were missing. -func TestTscMidCheckCancellation(t *testing.T) { +// TestTscCancellationAborts verifies that a canceled compile aborts with +// ExitStatusCanceled and never reports its (incomplete) diagnostics -- both when the +// signal arrives before the compile starts and when it lands mid-check, where a +// checker actually runs and is marked canceled. The mid-check cases are what would +// panic in checkNotCanceled if the reuse guards were missing (a canceled checker fed +// more files, or asked for global/declaration diagnostics again). +func TestTscCancellationAborts(t *testing.T) { t.Parallel() - // Many top-level statements per file so the checker polls ctx.Err() often and - // cancellation reliably lands mid-check, across more than one file. - var badStatements strings.Builder - for i := range 50 { - badStatements.WriteString("export const v") - badStatements.WriteString(strings.Repeat("x", i+1)) - badStatements.WriteString(`: number = "not a number";` + "\n") - } - badSrc := badStatements.String() - - // Inferred return types force the checker to serialize types during declaration - // emit (SerializeReturnTypeForSignature -> node reuse -> checkNotCanceled), a - // distinct reuse path from plain semantic checking. - var inferredSrc strings.Builder + // Many statements per file so a mid-check cancellation reliably lands while + // checking, across more than one file. The type errors let us tell whether the + // checker ran to completion: if it did the diagnostics are reported, if aborted + // they are not. Distinct names per statement keep the checker busy. + var bad, inferred strings.Builder for i := range 50 { - inferredSrc.WriteString("export function make") - inferredSrc.WriteString(strings.Repeat("x", i+1)) - inferredSrc.WriteString("() { return { a: 1, b: 'x', deep: [1, 2, 3] as const }; }\n") + x := strings.Repeat("x", i+1) + bad.WriteString("export const v" + x + `: number = "not a number";` + "\n") + // Inferred return types force type serialization during declaration emit + // (SerializeReturnTypeForSignature -> node reuse -> checkNotCanceled), a + // distinct checker-reuse path from plain semantic checking. + inferred.WriteString("export function make" + x + "() { return { a: 1, b: 'x', deep: [1, 2, 3] as const }; }\n") } + badSrc, inferredSrc := bad.String(), inferred.String() testCases := []struct { name string args []string tsconfig string src string + midCheck bool // cancel during checking rather than before the compile starts }{ { - // Single checker reused across files: after it cancels on an early file, - // forEachCheckerGroupDo must stop feeding it later files. - name: "single checker", + name: "pre-canceled single file", + args: []string{"--noEmit"}, + tsconfig: `{ "compilerOptions": { "noEmit": true, "strict": true } }`, + src: badSrc, + }, + { + name: "pre-canceled build mode", + args: []string{"-b"}, + tsconfig: `{ "compilerOptions": { "composite": true, "strict": true } }`, + src: badSrc, + }, + { + // --singleThreaded funnels all files through one checker, so after it + // cancels on an early file forEachCheckerGroupDo must stop feeding it later + // files. + name: "mid-check single checker", args: []string{"--noEmit", "--singleThreaded"}, tsconfig: `{ "compilerOptions": { "noEmit": true, "strict": true } }`, src: badSrc, + midCheck: true, }, { // tsc -b through the incremental program + orchestrator, which also reaches // GetGlobalDiagnostics from emitBuildInfo -> ensureHasErrorsForState. - name: "build mode", + name: "mid-check build mode", args: []string{"-b"}, tsconfig: `{ "compilerOptions": { "composite": true, "strict": true } }`, src: badSrc, + midCheck: true, }, { - // Declaration emit reuses checkers to serialize types; a canceled checker - // must not be handed to GetDeclarationDiagnostics. - name: "declaration emit", + // A canceled checker must not be handed to GetDeclarationDiagnostics. + name: "mid-check declaration emit", args: []string{"--noEmit", "--declaration", "--singleThreaded"}, tsconfig: `{ "compilerOptions": { "noEmit": true, "declaration": true, "strict": true } }`, - src: inferredSrc.String(), + src: inferredSrc, + midCheck: true, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - sys := newTestSys(&tscInput{ commandLineArgs: tc.args, files: FileMap{ @@ -196,40 +132,65 @@ func TestTscMidCheckCancellation(t *testing.T) { }, }, false) - // Let checking start, then cancel. The threshold is small relative to the - // number of statements so cancellation lands well before checking finishes. - ctx := newCancelAfterNPolls(5) - - resultCh := make(chan tsc.CommandLineResult, 1) - go func() { - resultCh <- execute.CommandLine(ctx, sys, tc.args, sys) - }() + result, midChecked := runWithCancellation(t, sys, tc.args, tc.midCheck) - select { - case result := <-resultCh: - // The run must abort (not run to completion) once canceled mid-check, - // and must not panic in checkNotCanceled. - if result.Status != tsc.ExitStatusCanceled { - t.Errorf("status = %v, want ExitStatusCanceled", result.Status) - } - // The cancellation must actually have landed mid-check, otherwise this - // test is not exercising what it claims. - if !ctx.tripped.Load() { - t.Error("expected cancellation to trip during checking, but it never did") - } - case <-time.After(30 * time.Second): - t.Fatal("compile did not abort after mid-check cancellation") + // Aborts with a distinct status (and must not panic reusing a canceled checker). + if result.Status != tsc.ExitStatusCanceled { + t.Errorf("status = %v, want ExitStatusCanceled", result.Status) + } + // Aborted checks must not report their incomplete diagnostics. + if out := sys.getOutput(true); strings.Contains(out, "error TS") { + t.Errorf("expected no diagnostics after cancellation; got output:\n%s", out) + } + // A mid-check case that never tripped during checking isn't testing what it claims. + if tc.midCheck && !midChecked { + t.Error("expected cancellation to trip during checking, but it never did") } }) } } -// TestTscCancellationSweep cancels at every successive point in the compile by -// increasing the poll threshold one step at a time. It walks cancellation past the -// check phase and into emit, covering windows a single fixed threshold would miss: -// the no-emit-on-error recheck that runs during emit, the emit-returns-nil path, and -// declaration-emit type serialization. At no threshold may the run panic, and it -// must always end Canceled or Success (if it finished before the trip). +// runWithCancellation runs the command line under a canceled context and returns the +// result. When midCheck is false the context is canceled before the run starts; when +// true it is canceled after checking has begun, and the returned bool reports whether +// that mid-check cancellation actually tripped. The run is guarded by a timeout so a +// regression that ignores cancellation fails loudly instead of hanging. +func runWithCancellation(t *testing.T, sys *TestSys, args []string, midCheck bool) (tsc.CommandLineResult, bool) { + t.Helper() + var ( + ctx context.Context + midChecked func() bool + ) + if midCheck { + // Threshold small relative to the statement count so cancellation lands well + // before checking finishes. + c := newCancelAfterNPolls(5) + ctx, midChecked = c, c.tripped.Load + } else { + canceled, cancel := context.WithCancel(context.Background()) + cancel() + ctx, midChecked = canceled, func() bool { return false } + } + + resultCh := make(chan tsc.CommandLineResult, 1) + go func() { + resultCh <- execute.CommandLine(ctx, sys, args, sys) + }() + select { + case result := <-resultCh: + return result, midChecked() + case <-time.After(30 * time.Second): + t.Fatal("compile did not abort after cancellation") + return tsc.CommandLineResult{}, false + } +} + +// TestTscCancellationSweep steps the cancellation point across the whole compile by +// increasing the poll threshold one step at a time, walking past the check phase and +// into emit. This covers windows a single fixed threshold would miss -- the +// no-emit-on-error recheck that runs during emit, the emit-returns-nil path, and +// declaration-emit type serialization -- and asserts that at no point does the run +// panic or report a partial result as success. func TestTscCancellationSweep(t *testing.T) { t.Parallel() From 66ce4e061bedd527bd2b18e4a49bc6b4f100143a Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sat, 11 Jul 2026 01:27:01 -0700 Subject: [PATCH 09/14] fix linter warning --- internal/execute/tsctests/tsccancel_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index ebb1839975a..0fca3bc0abd 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -63,11 +63,15 @@ func TestTscCancellationAborts(t *testing.T) { var bad, inferred strings.Builder for i := range 50 { x := strings.Repeat("x", i+1) - bad.WriteString("export const v" + x + `: number = "not a number";` + "\n") + bad.WriteString("export const v") + bad.WriteString(x) + bad.WriteString(`: number = "not a number";` + "\n") // Inferred return types force type serialization during declaration emit // (SerializeReturnTypeForSignature -> node reuse -> checkNotCanceled), a // distinct checker-reuse path from plain semantic checking. - inferred.WriteString("export function make" + x + "() { return { a: 1, b: 'x', deep: [1, 2, 3] as const }; }\n") + inferred.WriteString("export function make") + inferred.WriteString(x) + inferred.WriteString("() { return { a: 1, b: 'x', deep: [1, 2, 3] as const }; }\n") } badSrc, inferredSrc := bad.String(), inferred.String() From a4b8b541c0c5e321e0af38e30965f8990745c35c Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Sat, 11 Jul 2026 09:40:39 -0700 Subject: [PATCH 10/14] address copilot feedback --- internal/execute/build/buildtask.go | 26 +++++++++++--------------- internal/execute/build/orchestrator.go | 4 ++-- internal/execute/watcher.go | 2 +- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index b39e940fb15..80282dea5c1 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -74,16 +74,18 @@ type BuildTask struct { dirty bool } -func (t *BuildTask) waitOnUpstream(ctx context.Context) { +// Returns true when upstream is done, false when cancelled +func (t *BuildTask) waitOnUpstream(ctx context.Context) bool { for _, upstream := range t.upStream { select { case <-upstream.task.done: case <-ctx.Done(): // Canceled while waiting: stop blocking. buildProject observes the // cancellation and completes this task without building. - return + return false } } + return true } func (t *BuildTask) unblockDownstream() { @@ -129,12 +131,8 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { // Wait on upstream tasks to complete - t.waitOnUpstream(ctx) - // Canceled before we started the compile: skip the expensive work but still fall - // through to unblockDownstream so downstream and report waiters don't deadlock. An - // in-flight compile aborts on its own (the checker polls ctx) and surfaces - // ExitStatusCanceled via compileAndEmit. - if ctx.Err() != nil { + if !t.waitOnUpstream(ctx) { + // We were cancelled while waiting, just set the status and unblockDownstream t.result.exitStatus = tsc.ExitStatusCanceled } else if t.pending.Load() { t.status = t.getUpToDateStatus(orchestrator, path) @@ -165,14 +163,12 @@ func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator } func (t *BuildTask) updateDownstream(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { - // This seeds dependents' in-memory rebuild state (status, pending) from this - // project's result; it never touches emitted outputs. Skip it once canceled: the - // result may be from a compile that aborted mid-emit, and propagating its partial - // HasChangedDtsFile would corrupt downstream up-to-date decisions. + // Skip notifying downstream if we are canceled. Canceled builds may have partial results + // which could be otherwise handled improperly by downstream tasks. // - // Today this only fires in one-shot `tsc -b`, where downStream is empty and the - // body below is a no-op anyway. It becomes load-bearing once DoCycle threads a - // real (cancelable) context into watch rebuilds, where downStream is populated. + // In one-shot `tsc -b`, downStream is empty so the body below is a no-op. + // In watch mode, downStream is populated and this runs on subsequent rebuild cycles. + // Once DoCycle threads a cancelable context, the early-return here becomes load-bearing. if ctx.Err() != nil { return } diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index e68a6f92a7a..67c7b29666d 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -231,8 +231,8 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { o.GenerateGraph(nil) result := o.buildOrClean(ctx) if o.opts.Command.CompilerOptions.Watch.IsTrue() { - // If we were already cancelled return now, but treat it as success. - // in watch mode this is the only way to exit. + // If we were already canceled return now, but treat it as success. + // In watch mode this is the only way to exit. if result.Status == tsc.ExitStatusCanceled { result.Status = tsc.ExitStatusSuccess return result diff --git a/internal/execute/watcher.go b/internal/execute/watcher.go index fa92ed7cf33..5e8f0e7224d 100644 --- a/internal/execute/watcher.go +++ b/internal/execute/watcher.go @@ -527,7 +527,7 @@ func (w *Watcher) evictChangedSourceFiles(changedPaths map[string]fswatch.EventK } func (w *Watcher) compileAndEmit() tsc.CompileAndEmitResult { - // TODO: propagate a proper context here to better support cancelation + // TODO: propagate a proper context here to better support cancellation return tsc.EmitFilesAndReportErrors(context.Background(), tsc.EmitInput{ Sys: w.sys, ProgramLike: w.program, From 14c11e21d4938201303603093c09e6727acc93ac Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 23 Jul 2026 13:58:46 -0700 Subject: [PATCH 11/14] remove a context.Background, document the rest --- internal/execute/build/orchestrator.go | 15 +++++++++------ internal/execute/tsc.go | 6 +++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index 67c7b29666d..8bb62bacf9c 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -255,7 +255,7 @@ func (o *Orchestrator) Watch(ctx context.Context) { o.wm.EnsureDefaultBackend() } - o.updateWatch() + o.updateWatch(ctx) desiredDirs := o.computeDesiredWatches() if err := o.wm.ReconcileWatches(desiredDirs); err != nil { fmt.Fprintf(o.opts.Sys.Writer(), "%v\n", err) @@ -270,10 +270,10 @@ func (o *Orchestrator) Watch(ctx context.Context) { } } -func (o *Orchestrator) updateWatch() { +func (o *Orchestrator) updateWatch(ctx context.Context) { oldCache := o.host.mTimes o.host.mTimes = &collections.SyncMap[tspath.Path, time.Time]{} - o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { + o.rangeTask(ctx, func(_ context.Context, path tspath.Path, task *BuildTask) { task.updateWatch(o, oldCache) }) } @@ -398,6 +398,7 @@ func (o *Orchestrator) checkTasksForEventChanges(changedPaths map[string]fswatch for eventPath := range changedPaths { if o.host.FS().DirectoryExists(eventPath) { if o.wm.IsPathUnderWatch(eventPath, opts) { + // There is nothing interesting to cancel, just pass Background. o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { task.resetStatus() task.reportDone = make(chan struct{}) @@ -550,6 +551,7 @@ func (o *Orchestrator) DoCycle() { if overflow { // Overflow: reset all tasks to force a full rebuild. + // There is nothing interesting to cancel, just pass Background. o.rangeTask(context.Background(), func(_ context.Context, path tspath.Path, task *BuildTask) { task.resetConfig(o, path) task.reportDone = make(chan struct{}) @@ -573,9 +575,10 @@ func (o *Orchestrator) DoCycle() { o.GenerateGraphReusingOldTasks() } - // TODO: propagate a proper context here and support cancellation with cycle - o.buildOrClean(context.Background()) - o.updateWatch() + // TODO: propagate a proper context here and support cancellation within a cycle + ctx := context.Background() + o.buildOrClean(ctx) + o.updateWatch(ctx) desiredDirs := o.computeDesiredWatches() if err := o.wm.ReconcileWatches(desiredDirs); err != nil { fmt.Fprintf(o.opts.Sys.Writer(), "%v\n", err) diff --git a/internal/execute/tsc.go b/internal/execute/tsc.go index 07587bc64a7..8e21be29f18 100644 --- a/internal/execute/tsc.go +++ b/internal/execute/tsc.go @@ -55,15 +55,15 @@ func CommandLine(ctx context.Context, sys tsc.System, commandLineArgs []string, case "-b", "--b", "-build", "--build": return tscBuildCompilation(ctx, sys, tsoptions.ParseBuildCommandLine(commandLineArgs, sys), testing) // case "-f": - // return fmtMain(sys, commandLineArgs[1], commandLineArgs[1]) + // return fmtMain(ctx, sys, commandLineArgs[1], commandLineArgs[1]) } } return tscCompilation(ctx, sys, tsoptions.ParseCommandLine(commandLineArgs, sys), testing) } -func fmtMain(sys tsc.System, input, output string) tsc.ExitStatus { - ctx := format.WithFormatCodeSettings(context.Background(), lsutil.GetDefaultFormatCodeSettings(), "\n") +func fmtMain(ctx context.Context, sys tsc.System, input, output string) tsc.ExitStatus { + ctx = format.WithFormatCodeSettings(ctx, lsutil.GetDefaultFormatCodeSettings(), "\n") input = string(tspath.ToPath(input, sys.GetCurrentDirectory(), sys.FS().UseCaseSensitiveFileNames())) output = string(tspath.ToPath(output, sys.GetCurrentDirectory(), sys.FS().UseCaseSensitiveFileNames())) fileContent, ok := sys.FS().ReadFile(input) From 0e3658f2eb8c92c3d78d6c7b52c7d703be1c5f01 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 23 Jul 2026 22:49:37 -0700 Subject: [PATCH 12/14] address review feedback --- cmd/tsgo/main.go | 10 +-- cmd/tsgo/reraisesignal_other.go | 13 ++++ cmd/tsgo/reraisesignal_unix.go | 27 +++++++ internal/execute/build/buildtask.go | 22 ++++-- internal/execute/build/orchestrator.go | 2 +- internal/execute/tsctests/tsccancel_test.go | 79 +++++++++++++++++++++ 6 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 cmd/tsgo/reraisesignal_other.go create mode 100644 cmd/tsgo/reraisesignal_unix.go diff --git a/cmd/tsgo/main.go b/cmd/tsgo/main.go index d5c510f3141..c45ae710400 100644 --- a/cmd/tsgo/main.go +++ b/cmd/tsgo/main.go @@ -52,13 +52,13 @@ func runMain() int { if result.Status == tsc.ExitStatusCanceled { // A signal canceled the run. Re-raise it so we terminate via the signal itself, // yielding the conventional exit code (130 for SIGINT, 143 for SIGTERM) and the - // terminal reset an unhandled signal would produce. + // terminal reset an unhandled signal would produce. On platforms that cannot + // re-deliver the signal (e.g. Windows), reRaiseSignal returns 0 and we exit with + // the numeric status instead. select { case sig := <-canceledBy: - if s, ok := sig.(syscall.Signal); ok { - signal.Reset(s) - _ = syscall.Kill(os.Getpid(), s) - return 128 + int(s) // fallback in case the signal doesn't land promptly + if signo := reRaiseSignal(sig); signo != 0 { + return 128 + signo // fallback in case the signal doesn't land promptly } default: } diff --git a/cmd/tsgo/reraisesignal_other.go b/cmd/tsgo/reraisesignal_other.go new file mode 100644 index 00000000000..c89b757d108 --- /dev/null +++ b/cmd/tsgo/reraisesignal_other.go @@ -0,0 +1,13 @@ +//go:build !unix + +package main + +import "os" + +// reRaiseSignal is a no-op on platforms that cannot re-deliver a termination +// signal to the current process (notably Windows, where os.Process.Signal does +// not implement Interrupt). It always returns 0 so the caller falls back to +// exiting with a numeric status. +func reRaiseSignal(sig os.Signal) int { + return 0 +} diff --git a/cmd/tsgo/reraisesignal_unix.go b/cmd/tsgo/reraisesignal_unix.go new file mode 100644 index 00000000000..94b8e817b74 --- /dev/null +++ b/cmd/tsgo/reraisesignal_unix.go @@ -0,0 +1,27 @@ +//go:build unix + +package main + +import ( + "os" + "os/signal" + "syscall" +) + +// reRaiseSignal resets the default disposition for sig and re-delivers it to +// this process, so we terminate via the signal itself (yielding the +// conventional 128+signo exit code and the terminal reset an unhandled signal +// produces). It returns the number of the signal that was re-raised, or 0 if +// sig is not a signal this platform can re-deliver, in which case the caller +// should fall back to exiting with a numeric status. +func reRaiseSignal(sig os.Signal) int { + s, ok := sig.(syscall.Signal) + if !ok { + return 0 + } + signal.Reset(s) + if proc, err := os.FindProcess(os.Getpid()); err == nil { + _ = proc.Signal(s) + } + return int(s) +} diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index 80282dea5c1..daad7e69fbf 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -130,9 +130,13 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b } func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { - // Wait on upstream tasks to complete - if !t.waitOnUpstream(ctx) { - // We were cancelled while waiting, just set the status and unblockDownstream + // Honor cancellation before doing any work. waitOnUpstream only observes the + // context while it has upstream tasks to wait on, so a task with no upstream + // (e.g. a root project that is already up to date) would otherwise take the + // no-build success path and swallow an interrupt that arrived before we started. + if ctx.Err() != nil || !t.waitOnUpstream(ctx) { + // Canceled before starting or while waiting on upstream: set the status and + // unblockDownstream without building. t.result.exitStatus = tsc.ExitStatusCanceled } else if t.pending.Load() { t.status = t.getUpToDateStatus(orchestrator, path) @@ -739,7 +743,7 @@ func (t *BuildTask) updateTimeStamps(orchestrator *Orchestrator, emittedFiles [] updateTimeStamp(t.resolved.GetBuildInfoFileName()) } -func (t *BuildTask) cleanProject(orchestrator *Orchestrator, path tspath.Path) { +func (t *BuildTask) cleanProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { if t.resolved == nil { t.reportDiagnostic(ast.NewCompilerDiagnostic(diagnostics.File_0_not_found, t.config)) t.result.exitStatus = tsc.ExitStatusDiagnosticsPresent_OutputsSkipped @@ -748,8 +752,18 @@ func (t *BuildTask) cleanProject(orchestrator *Orchestrator, path tspath.Path) { inputs := collections.NewSetFromItems(core.Map(t.resolved.FileNames(), orchestrator.toPath)...) for outputFile := range t.resolved.GetOutputFileNames() { + // Stop deleting outputs if we were canceled. Report cancellation so the CLI + // can re-raise the signal rather than exiting with success mid-clean. + if ctx.Err() != nil { + t.result.exitStatus = tsc.ExitStatusCanceled + return + } t.cleanProjectOutput(orchestrator, outputFile, inputs) } + if ctx.Err() != nil { + t.result.exitStatus = tsc.ExitStatusCanceled + return + } t.cleanProjectOutput(orchestrator, t.resolved.GetBuildInfoFileName(), inputs) } diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index 8bb62bacf9c..bbdfedfa230 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -665,7 +665,7 @@ func (o *Orchestrator) buildOrCleanProject(ctx context.Context, task *BuildTask, if !o.opts.Command.BuildOptions.Clean.IsTrue() { task.buildProject(ctx, o, path) } else { - task.cleanProject(o, path) + task.cleanProject(ctx, o, path) } task.report(o, path, buildResult) } diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index 0fca3bc0abd..70a90ae1e83 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -189,6 +189,85 @@ func runWithCancellation(t *testing.T, sys *TestSys, args []string, midCheck boo } } +// runPreCanceled runs the command line on an existing sys under a context that is +// already canceled before the run starts, guarded by a timeout so a regression that +// ignores cancellation fails loudly instead of hanging. +func runPreCanceled(t *testing.T, sys *TestSys, args []string) tsc.CommandLineResult { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + resultCh := make(chan tsc.CommandLineResult, 1) + go func() { + resultCh <- execute.CommandLine(ctx, sys, args, sys) + }() + select { + case result := <-resultCh: + return result + case <-time.After(30 * time.Second): + t.Fatal("run did not abort after cancellation") + return tsc.CommandLineResult{} + } +} + +// TestTscBuildCancellationUpToDate verifies that an interrupt is honored even when a +// `tsc -b` build has nothing to do. A root project that is already up to date has no +// upstream to wait on, so the no-build path must still observe a pre-canceled context +// and report ExitStatusCanceled rather than swallowing the interrupt as success. +func TestTscBuildCancellationUpToDate(t *testing.T) { + t.Parallel() + files := FileMap{ + "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "composite": true } }`, + "/home/src/workspaces/project/a.ts": "export const a = 1;\n", + } + sys := newTestSys(&tscInput{ + commandLineArgs: []string{"-b"}, + files: files, + }, false) + + // First build to success so the project is up to date on the next run. + if result := execute.CommandLine(context.Background(), sys, []string{"-b"}, sys); result.Status != tsc.ExitStatusSuccess { + t.Fatalf("initial build status = %v, want ExitStatusSuccess", result.Status) + } + + // A second, pre-canceled build has nothing to build; it must still report canceled. + result := runPreCanceled(t, sys, []string{"-b"}) + if result.Status != tsc.ExitStatusCanceled { + t.Errorf("status = %v, want ExitStatusCanceled", result.Status) + } +} + +// TestTscCleanCancellation verifies that `tsc -b --clean` interrupted before it runs +// does not delete outputs and reports ExitStatusCanceled instead of success. +func TestTscCleanCancellation(t *testing.T) { + t.Parallel() + const outFile = "/home/src/workspaces/project/a.js" + files := FileMap{ + "/home/src/workspaces/project/tsconfig.json": `{ "compilerOptions": { "composite": true } }`, + "/home/src/workspaces/project/a.ts": "export const a = 1;\n", + } + sys := newTestSys(&tscInput{ + commandLineArgs: []string{"-b"}, + files: files, + }, false) + + // Build first so there is an output for clean to (potentially) delete. + if result := execute.CommandLine(context.Background(), sys, []string{"-b"}, sys); result.Status != tsc.ExitStatusSuccess { + t.Fatalf("initial build status = %v, want ExitStatusSuccess", result.Status) + } + if !sys.FS().FileExists(outFile) { + t.Fatalf("expected %s to exist after build", outFile) + } + + // A pre-canceled clean must abort before deleting outputs and report canceled. + result := runPreCanceled(t, sys, []string{"-b", "--clean"}) + if result.Status != tsc.ExitStatusCanceled { + t.Errorf("status = %v, want ExitStatusCanceled", result.Status) + } + if !sys.FS().FileExists(outFile) { + t.Errorf("expected %s to survive a canceled clean", outFile) + } +} + // TestTscCancellationSweep steps the cancellation point across the whole compile by // increasing the poll threshold one step at a time, walking past the check phase and // into emit. This covers windows a single fixed threshold would miss -- the From 1ed1e1302af60be981fc1f809ee0d416b48c8175 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Tue, 11 Aug 2026 19:35:08 -0700 Subject: [PATCH 13/14] address more copilot feedback --- cmd/tsgo/main.go | 30 ++-- cmd/tsgo/reraisesignal_other.go | 9 +- cmd/tsgo/reraisesignal_unix.go | 13 +- internal/compiler/checkerpool.go | 7 +- internal/compiler/checkerpool_test.go | 21 ++- internal/compiler/program.go | 9 +- internal/execute/build/buildtask.go | 26 +--- internal/execute/build/graph_test.go | 6 +- internal/execute/build/orchestrator.go | 52 ++++--- internal/execute/tsc/emit.go | 8 +- internal/execute/tsctests/tsccancel_test.go | 132 +++++++++++------- .../execute/tsctests/watcher_race_test.go | 5 +- 12 files changed, 172 insertions(+), 146 deletions(-) diff --git a/cmd/tsgo/main.go b/cmd/tsgo/main.go index c45ae710400..1abf26d9bbb 100644 --- a/cmd/tsgo/main.go +++ b/cmd/tsgo/main.go @@ -27,16 +27,14 @@ func runMain() int { } } - // Use signal.Notify with our own channel rather than signal.NotifyContext: the - // latter's context can't tell us which signal fired, and we need it to exit like - // the JS tsc, which installs no handler and lets node terminate via the signal. + // Not signal.NotifyContext: we need to know which signal fired so we can exit by + // re-raising it, the way the JS tsc terminates under node's default handler. sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) defer signal.Stop(sigCh) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - // canceledBy carries the interrupting signal (if any) to the code below. It is - // written before cancel() and thus before CommandLine can observe cancellation. + // Written before cancel(), so a canceled CommandLine always finds the signal here. canceledBy := make(chan os.Signal, 1) go func() { select { @@ -50,18 +48,26 @@ func runMain() int { result := execute.CommandLine(ctx, newSystem(), args, nil) if result.Status == tsc.ExitStatusCanceled { - // A signal canceled the run. Re-raise it so we terminate via the signal itself, - // yielding the conventional exit code (130 for SIGINT, 143 for SIGTERM) and the - // terminal reset an unhandled signal would produce. On platforms that cannot - // re-deliver the signal (e.g. Windows), reRaiseSignal returns 0 and we exit with - // the numeric status instead. + // Terminate via the signal itself, for the conventional exit code (130 for + // SIGINT, 143 for SIGTERM) and the terminal reset an unhandled signal produces. select { case sig := <-canceledBy: - if signo := reRaiseSignal(sig); signo != 0 { - return 128 + signo // fallback in case the signal doesn't land promptly + // Does not return if the signal is re-delivered; otherwise (e.g. Windows) + // fall through to the same exit code numerically. + reRaiseSignal(sig) + if signo := signalNumber(sig); signo != 0 { + return 128 + signo } default: } } return int(result.Status) } + +// signalNumber returns the platform signal number for sig, or 0 if it has none. +func signalNumber(sig os.Signal) int { + if s, ok := sig.(syscall.Signal); ok { + return int(s) + } + return 0 +} diff --git a/cmd/tsgo/reraisesignal_other.go b/cmd/tsgo/reraisesignal_other.go index c89b757d108..f7bff55bb47 100644 --- a/cmd/tsgo/reraisesignal_other.go +++ b/cmd/tsgo/reraisesignal_other.go @@ -4,10 +4,7 @@ package main import "os" -// reRaiseSignal is a no-op on platforms that cannot re-deliver a termination -// signal to the current process (notably Windows, where os.Process.Signal does -// not implement Interrupt). It always returns 0 so the caller falls back to -// exiting with a numeric status. -func reRaiseSignal(sig os.Signal) int { - return 0 +// reRaiseSignal is a no-op here: these platforms cannot re-deliver a termination +// signal to the current process (on Windows, os.Process.Signal rejects Interrupt). +func reRaiseSignal(sig os.Signal) { } diff --git a/cmd/tsgo/reraisesignal_unix.go b/cmd/tsgo/reraisesignal_unix.go index 94b8e817b74..366b4ce5201 100644 --- a/cmd/tsgo/reraisesignal_unix.go +++ b/cmd/tsgo/reraisesignal_unix.go @@ -8,20 +8,15 @@ import ( "syscall" ) -// reRaiseSignal resets the default disposition for sig and re-delivers it to -// this process, so we terminate via the signal itself (yielding the -// conventional 128+signo exit code and the terminal reset an unhandled signal -// produces). It returns the number of the signal that was re-raised, or 0 if -// sig is not a signal this platform can re-deliver, in which case the caller -// should fall back to exiting with a numeric status. -func reRaiseSignal(sig os.Signal) int { +// reRaiseSignal restores the default disposition for sig and re-delivers it to this +// process, terminating us via the signal itself. It returns only if that fails. +func reRaiseSignal(sig os.Signal) { s, ok := sig.(syscall.Signal) if !ok { - return 0 + return } signal.Reset(s) if proc, err := os.FindProcess(os.Getpid()); err == nil { _ = proc.Signal(s) } - return int(s) } diff --git a/internal/compiler/checkerpool.go b/internal/compiler/checkerpool.go index 13e1ac05fae..f3f204aa624 100644 --- a/internal/compiler/checkerpool.go +++ b/internal/compiler/checkerpool.go @@ -137,8 +137,7 @@ func (p *checkerPool) GetGlobalDiagnostics() []*ast.Diagnostic { p.createCheckers() globalDiagnostics := make([][]*ast.Diagnostic, len(p.checkers)) p.forEachCheckerParallel(func(idx int, checker *checker.Checker) { - // A canceled checker panics if asked for diagnostics (checkNotCanceled), and - // its results are discarded once canceled anyway. Skip it. + // A canceled checker panics in checkNotCanceled if asked for diagnostics. if checker.WasCanceled() { return } @@ -160,8 +159,8 @@ func (p *checkerPool) forEachCheckerGroupDo(ctx context.Context, files []*ast.So p.locks[checkerIdx].Lock() defer p.locks[checkerIdx].Unlock() for i, file := range files { - // Stop once canceled: feeding another file to a checker that already - // canceled mid-check would panic in checkNotCanceled. + // Feeding another file to a checker that canceled mid-check panics in + // checkNotCanceled. if ctx.Err() != nil { break } diff --git a/internal/compiler/checkerpool_test.go b/internal/compiler/checkerpool_test.go index 4a31da6f6d3..9156124e885 100644 --- a/internal/compiler/checkerpool_test.go +++ b/internal/compiler/checkerpool_test.go @@ -13,9 +13,8 @@ import ( "github.com/microsoft/typescript-go/internal/vfs/vfstest" ) -// cancelAfterNPolls reports itself canceled only after Err has been polled -// pollThreshold times while still uncanceled, so cancellation lands after checking -// has begun rather than before it starts. Once tripped it stays canceled. +// cancelAfterNPolls cancels itself after Err has been polled pollThreshold times, +// then stays canceled, so cancellation lands after checking has begun. type cancelAfterNPolls struct { context.Context pollThreshold int32 @@ -43,11 +42,10 @@ func (c *cancelAfterNPolls) Err() error { func (c *cancelAfterNPolls) Done() <-chan struct{} { return c.done } -// TestGetGlobalDiagnosticsAfterCancellation pins the checker-pool behavior that a -// checker canceled mid-check is skipped by GetGlobalDiagnostics rather than reused -// (which panics in checkNotCanceled). This is the source-level guard that protects -// every GetGlobalDiagnostics caller, including emitBuildInfo's error-state probe, -// which has no surrounding cancellation check. +// TestGetGlobalDiagnosticsAfterCancellation pins the checker-pool behavior that +// GetGlobalDiagnostics skips a checker canceled mid-check rather than reusing it +// (which panics in checkNotCanceled). This guards every caller, including +// emitBuildInfo's error-state probe, which has no cancellation check of its own. func TestGetGlobalDiagnosticsAfterCancellation(t *testing.T) { t.Parallel() @@ -69,8 +67,8 @@ func TestGetGlobalDiagnosticsAfterCancellation(t *testing.T) { _ = fs.WriteFile(name, src.String()) } - // One checker so a single mid-check cancellation marks the checker the subsequent - // GetGlobalDiagnostics will visit. + // One checker, so the mid-check cancellation marks the same checker that the + // subsequent GetGlobalDiagnostics will visit. oneChecker := 1 program := compiler.NewProgram(compiler.ProgramOptions{ Config: &tsoptions.ParsedCommandLine{ @@ -84,8 +82,7 @@ func TestGetGlobalDiagnosticsAfterCancellation(t *testing.T) { ctx := newCancelAfterNPolls(5) - // Drive checking under the canceling context to cancel the checker. Discard the - // (partial) result; we only care that the checker is now canceled. + // Drive checking only to leave the checker canceled; the result is discarded. _ = program.GetSemanticDiagnostics(ctx, nil) if !ctx.tripped.Load() { t.Fatal("expected cancellation to trip during checking, but it never did") diff --git a/internal/compiler/program.go b/internal/compiler/program.go index 16d1858cb95..de74ef46da2 100644 --- a/internal/compiler/program.go +++ b/internal/compiler/program.go @@ -1762,8 +1762,8 @@ func HandleNoEmitOnError(ctx context.Context, program ProgramLike, files []*ast. return nil // No emit on error is not set, so we can proceed with emitting } if ctx.Err() != nil { - // Canceled: don't re-run diagnostics on checkers that may already be canceled - // (checkNotCanceled would panic). The emit is being abandoned regardless. + // The emit is abandoned anyway, and re-running diagnostics on an + // already-canceled checker panics in checkNotCanceled. return nil } @@ -1820,9 +1820,8 @@ func GetDiagnosticsOfAnyProgram( if len(allDiagnostics) == configFileParsingDiagnosticsLength { allDiagnostics = appendDiagnosticsForAllFiles(allDiagnostics, getSemanticDiagnostics) - // Stop once canceled: the diagnostics are discarded anyway, and the calls - // below (GetGlobalDiagnostics, GetDeclarationDiagnostics) reuse the now - // canceled checkers, which panics in checkNotCanceled. + // The calls below reuse the now-canceled checkers, which panics in + // checkNotCanceled; the diagnostics are discarded anyway. if ctx.Err() != nil { return allDiagnostics } diff --git a/internal/execute/build/buildtask.go b/internal/execute/build/buildtask.go index daad7e69fbf..22ad52e3cbc 100644 --- a/internal/execute/build/buildtask.go +++ b/internal/execute/build/buildtask.go @@ -74,14 +74,12 @@ type BuildTask struct { dirty bool } -// Returns true when upstream is done, false when cancelled +// Returns true when upstream is done, false when canceled. func (t *BuildTask) waitOnUpstream(ctx context.Context) bool { for _, upstream := range t.upStream { select { case <-upstream.task.done: case <-ctx.Done(): - // Canceled while waiting: stop blocking. buildProject observes the - // cancellation and completes this task without building. return false } } @@ -130,13 +128,10 @@ func (t *BuildTask) report(orchestrator *Orchestrator, configPath tspath.Path, b } func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { - // Honor cancellation before doing any work. waitOnUpstream only observes the - // context while it has upstream tasks to wait on, so a task with no upstream - // (e.g. a root project that is already up to date) would otherwise take the - // no-build success path and swallow an interrupt that arrived before we started. + // The ctx.Err() check is not redundant: waitOnUpstream only observes the context + // while it has upstream to wait on, so a task with none (e.g. an up-to-date root) + // would take the no-build success path and swallow the interrupt. if ctx.Err() != nil || !t.waitOnUpstream(ctx) { - // Canceled before starting or while waiting on upstream: set the status and - // unblockDownstream without building. t.result.exitStatus = tsc.ExitStatusCanceled } else if t.pending.Load() { t.status = t.getUpToDateStatus(orchestrator, path) @@ -167,12 +162,7 @@ func (t *BuildTask) buildProject(ctx context.Context, orchestrator *Orchestrator } func (t *BuildTask) updateDownstream(ctx context.Context, orchestrator *Orchestrator, path tspath.Path) { - // Skip notifying downstream if we are canceled. Canceled builds may have partial results - // which could be otherwise handled improperly by downstream tasks. - // - // In one-shot `tsc -b`, downStream is empty so the body below is a no-op. - // In watch mode, downStream is populated and this runs on subsequent rebuild cycles. - // Once DoCycle threads a cancelable context, the early-return here becomes load-bearing. + // A canceled build has partial results; downstream tasks must not consume them. if ctx.Err() != nil { return } @@ -258,8 +248,7 @@ func (t *BuildTask) compileAndEmit(ctx context.Context, orchestrator *Orchestrat t.result.exitStatus = result.Status t.result.statistics = statistics if result.Status == tsc.ExitStatusCanceled { - // Canceled: the result is incomplete (EmitResult is nil). Leave the task - // partial on purpose. Only ExitStatusCanceled propagates. + // EmitResult is nil when canceled; the code below would dereference it. return } t.packageJsons = t.result.program.PackageJsonLookupPaths() @@ -752,8 +741,7 @@ func (t *BuildTask) cleanProject(ctx context.Context, orchestrator *Orchestrator inputs := collections.NewSetFromItems(core.Map(t.resolved.FileNames(), orchestrator.toPath)...) for outputFile := range t.resolved.GetOutputFileNames() { - // Stop deleting outputs if we were canceled. Report cancellation so the CLI - // can re-raise the signal rather than exiting with success mid-clean. + // Stop mid-clean rather than exiting with success, so the CLI re-raises. if ctx.Err() != nil { t.result.exitStatus = tsc.ExitStatusCanceled return diff --git a/internal/execute/build/graph_test.go b/internal/execute/build/graph_test.go index c0288f94868..448b39a887b 100644 --- a/internal/execute/build/graph_test.go +++ b/internal/execute/build/graph_test.go @@ -115,7 +115,7 @@ func (b *buildOrderTestCase) run(t *testing.T) { Sys: sys, Command: buildCommand, }) - orchestrator.GenerateGraph(nil) + orchestrator.GenerateGraph(t.Context(), nil) buildOrder := core.Map(orchestrator.Order(), b.projectName) assert.DeepEqual(t, buildOrder, b.expected) verifyDeps(orchestrator, buildOrder, false) @@ -136,7 +136,7 @@ func (b *buildOrderTestCase) run(t *testing.T) { } } - orchestrator.GenerateGraphReusingOldTasks() + orchestrator.GenerateGraphReusingOldTasks(t.Context()) buildOrder2 := core.Map(orchestrator.Order(), b.projectName) assert.DeepEqual(t, buildOrder2, b.expected) @@ -146,7 +146,7 @@ func (b *buildOrderTestCase) run(t *testing.T) { Sys: sys, Command: buildCommandWatch, }) - orchestrator.GenerateGraph(nil) + orchestrator.GenerateGraph(t.Context(), nil) buildOrder3 := core.Map(orchestrator.Order(), b.projectName) verifyDeps(orchestrator, buildOrder3, true) }) diff --git a/internal/execute/build/orchestrator.go b/internal/execute/build/orchestrator.go index bbdfedfa230..0aefdf75a52 100644 --- a/internal/execute/build/orchestrator.go +++ b/internal/execute/build/orchestrator.go @@ -121,9 +121,13 @@ func (o *Orchestrator) getTask(path tspath.Path) *BuildTask { return task } -func (o *Orchestrator) createBuildTasks(oldTasks *collections.SyncMap[tspath.Path, *BuildTask], configs []string, wg core.WorkGroup) { +func (o *Orchestrator) createBuildTasks(ctx context.Context, oldTasks *collections.SyncMap[tspath.Path, *BuildTask], configs []string, wg core.WorkGroup) { for _, config := range configs { wg.Queue(func() { + if ctx.Err() != nil { + // Stop early when canceled, our caller will report status. + return + } path := o.toPath(config) var task *BuildTask var buildInfo *buildInfoEntry @@ -148,7 +152,7 @@ func (o *Orchestrator) createBuildTasks(oldTasks *collections.SyncMap[tspath.Pat task.resolved = o.host.GetResolvedProjectReference(config, path) task.upStream = nil if task.resolved != nil { - o.createBuildTasks(oldTasks, task.resolved.ResolvedProjectReferencePaths(), wg) + o.createBuildTasks(ctx, oldTasks, task.resolved.ResolvedProjectReferencePaths(), wg) } }) } @@ -200,21 +204,28 @@ func (o *Orchestrator) setupBuildTask( return task } -func (o *Orchestrator) GenerateGraphReusingOldTasks() { +func (o *Orchestrator) GenerateGraphReusingOldTasks(ctx context.Context) { tasks := o.tasks o.tasks = &collections.SyncMap[tspath.Path, *BuildTask]{} o.order = nil o.errors = nil - o.GenerateGraph(tasks) + o.GenerateGraph(ctx, tasks) } -func (o *Orchestrator) GenerateGraph(oldTasks *collections.SyncMap[tspath.Path, *BuildTask]) { +func (o *Orchestrator) GenerateGraph(ctx context.Context, oldTasks *collections.SyncMap[tspath.Path, *BuildTask]) { projects := o.opts.Command.ResolvedProjectPaths() // Parse all config files in parallel wg := core.NewWorkGroup(o.opts.Command.CompilerOptions.SingleThreaded.IsTrue()) - o.createBuildTasks(oldTasks, projects, wg) + o.createBuildTasks(ctx, oldTasks, projects, wg) wg.RunAndWait() + // Cancellation leaves tasks missing for projects we never resolved, and + // setupBuildTask panics on a referenced project that has none. Leave the order + // empty; buildOrClean turns that into ExitStatusCanceled. + if ctx.Err() != nil { + return + } + // Generate the graph completed := collections.Set[tspath.Path]{} analyzing := collections.Set[tspath.Path]{} @@ -228,11 +239,10 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { if o.opts.Command.CompilerOptions.Watch.IsTrue() { o.watchStatusReporter(ast.NewCompilerDiagnostic(diagnostics.Starting_compilation_in_watch_mode)) } - o.GenerateGraph(nil) + o.GenerateGraph(ctx, nil) result := o.buildOrClean(ctx) if o.opts.Command.CompilerOptions.Watch.IsTrue() { - // If we were already canceled return now, but treat it as success. - // In watch mode this is the only way to exit. + // Cancellation is the only way to exit a watch, so it is not a failure. if result.Status == tsc.ExitStatusCanceled { result.Status = tsc.ExitStatusSuccess return result @@ -241,7 +251,6 @@ func (o *Orchestrator) Start(ctx context.Context) tsc.CommandLineResult { result.Watcher = o return result } - // Non-watch `tsc -b`: honor cancellation so a long build responds to SIGINT. return result } @@ -570,13 +579,14 @@ func (o *Orchestrator) DoCycle() { } o.watchStatusReporter(ast.NewCompilerDiagnostic(diagnostics.File_change_detected_Starting_incremental_compilation)) + + // TODO: propagate a proper context here and support cancellation within a cycle + ctx := context.Background() if needsConfigUpdate.Load() { // Generate new tasks - o.GenerateGraphReusingOldTasks() + o.GenerateGraphReusingOldTasks(ctx) } - // TODO: propagate a proper context here and support cancellation within a cycle - ctx := context.Background() o.buildOrClean(ctx) o.updateWatch(ctx) desiredDirs := o.computeDesiredWatches() @@ -589,6 +599,11 @@ func (o *Orchestrator) DoCycle() { } func (o *Orchestrator) buildOrClean(ctx context.Context) tsc.CommandLineResult { + // Cancellation during GenerateGraph leaves the order empty, so no task exists to + // report the canceled status. + if ctx.Err() != nil && len(o.order) == 0 { + return tsc.CommandLineResult{Status: tsc.ExitStatusCanceled} + } if !o.opts.Command.BuildOptions.Clean.IsTrue() && o.opts.Command.BuildOptions.Verbose.IsTrue() { o.createBuilderStatusReporter(nil)(ast.NewCompilerDiagnostic( diagnostics.Projects_in_this_build_Colon_0, @@ -603,8 +618,8 @@ func (o *Orchestrator) buildOrClean(ctx context.Context) tsc.CommandLineResult { o.rangeTask(ctx, func(ctx context.Context, path tspath.Path, task *BuildTask) { o.buildOrCleanProject(ctx, task, path, &buildResult) }) - // A canceled task surfaces ExitStatusCanceled through its own report(), so the - // aggregated status reflects cancellation without a separate override here. + // No cancellation override needed here: a canceled task reports + // ExitStatusCanceled through its own report(). } else { // Circularity errors prevent any project from being built buildResult.result.Status = tsc.ExitStatusProjectReferenceCycle_OutputsSkipped @@ -639,10 +654,9 @@ func (o *Orchestrator) rangeTask(ctx context.Context, f func(ctx context.Context } runTask := func() { for path, task, ok := getNextTask(); ok; path, task, ok = getNextTask() { - // f is called for every task, even after cancellation: each task must - // complete its lifecycle (close its done/reportDone channels) or the - // upstream/report waiters of other tasks deadlock. Cancellation is observed - // inside buildProject, which skips the compile but still completes the task. + // Every task runs even after cancellation: each must close its + // done/reportDone channels or other tasks' waiters deadlock. buildProject + // skips the compile but still completes the task. f(ctx, path, task) } } diff --git a/internal/execute/tsc/emit.go b/internal/execute/tsc/emit.go index c6fcb3ed471..89e26a6f64e 100644 --- a/internal/execute/tsc/emit.go +++ b/internal/execute/tsc/emit.go @@ -111,8 +111,7 @@ func EmitFilesAndReportErrors(ctx context.Context, input EmitInput) (result Comp }, ) - // On cancellation the diagnostics above are incomplete; abort rather than emit - // or report them as a complete result. + // The diagnostics above are incomplete when canceled; do not emit or report them. if ctx.Err() != nil { result.Status = ExitStatusCanceled return result @@ -125,9 +124,8 @@ func EmitFilesAndReportErrors(ctx context.Context, input EmitInput) (result Comp WriteFile: input.WriteFile, }) result.times.emitTime += input.Sys.Now().Sub(emitStart) - // Emit returns nil if it was canceled partway through (e.g. cancellation - // during the internal no-emit-on-error recheck). Abort rather than report a - // nil result as success. + // Emit returns nil if canceled partway through (e.g. during its internal + // no-emit-on-error recheck), which must not be reported as success. if ctx.Err() != nil { result.Status = ExitStatusCanceled return result diff --git a/internal/execute/tsctests/tsccancel_test.go b/internal/execute/tsctests/tsccancel_test.go index 70a90ae1e83..9dd676f50af 100644 --- a/internal/execute/tsctests/tsccancel_test.go +++ b/internal/execute/tsctests/tsccancel_test.go @@ -2,6 +2,7 @@ package tsctests import ( "context" + "fmt" "strings" "sync/atomic" "testing" @@ -11,11 +12,10 @@ import ( "github.com/microsoft/typescript-go/internal/execute/tsc" ) -// cancelAfterNPolls is a context that reports itself canceled only after Err has -// been polled pollThreshold times while still uncanceled. The checker polls -// ctx.Err() between top-level statements (checkSourceElements), so a small threshold -// lands the cancellation *after* checking has begun rather than before it starts -- -// the case a pre-canceled context cannot exercise. Once tripped it stays canceled. +// cancelAfterNPolls is a context that cancels itself after Err has been polled +// pollThreshold times, then stays canceled. The checker polls between top-level +// statements (checkSourceElements), so the threshold selects how far into checking +// the cancellation lands -- something a pre-canceled context cannot exercise. type cancelAfterNPolls struct { context.Context pollThreshold int32 @@ -48,18 +48,15 @@ func (c *cancelAfterNPolls) Err() error { func (c *cancelAfterNPolls) Done() <-chan struct{} { return c.done } // TestTscCancellationAborts verifies that a canceled compile aborts with -// ExitStatusCanceled and never reports its (incomplete) diagnostics -- both when the -// signal arrives before the compile starts and when it lands mid-check, where a -// checker actually runs and is marked canceled. The mid-check cases are what would -// panic in checkNotCanceled if the reuse guards were missing (a canceled checker fed -// more files, or asked for global/declaration diagnostics again). +// ExitStatusCanceled and never reports its incomplete diagnostics. Without the +// checker-reuse guards, the mid-check cases panic in checkNotCanceled -- a canceled +// checker fed more files, or asked for global/declaration diagnostics again. func TestTscCancellationAborts(t *testing.T) { t.Parallel() - // Many statements per file so a mid-check cancellation reliably lands while - // checking, across more than one file. The type errors let us tell whether the - // checker ran to completion: if it did the diagnostics are reported, if aborted - // they are not. Distinct names per statement keep the checker busy. + // Many distinctly-named statements per file, so a mid-check cancellation reliably + // lands while checking. The type errors are the signal: reported means the checker + // ran to completion, absent means it aborted. var bad, inferred strings.Builder for i := range 50 { x := strings.Repeat("x", i+1) @@ -95,9 +92,8 @@ func TestTscCancellationAborts(t *testing.T) { src: badSrc, }, { - // --singleThreaded funnels all files through one checker, so after it - // cancels on an early file forEachCheckerGroupDo must stop feeding it later - // files. + // One checker for all files, so forEachCheckerGroupDo must stop feeding it + // files after it cancels on an early one. name: "mid-check single checker", args: []string{"--noEmit", "--singleThreaded"}, tsconfig: `{ "compilerOptions": { "noEmit": true, "strict": true } }`, @@ -138,15 +134,13 @@ func TestTscCancellationAborts(t *testing.T) { result, midChecked := runWithCancellation(t, sys, tc.args, tc.midCheck) - // Aborts with a distinct status (and must not panic reusing a canceled checker). if result.Status != tsc.ExitStatusCanceled { t.Errorf("status = %v, want ExitStatusCanceled", result.Status) } - // Aborted checks must not report their incomplete diagnostics. if out := sys.getOutput(true); strings.Contains(out, "error TS") { t.Errorf("expected no diagnostics after cancellation; got output:\n%s", out) } - // A mid-check case that never tripped during checking isn't testing what it claims. + // A mid-check case that never tripped isn't testing what it claims. if tc.midCheck && !midChecked { t.Error("expected cancellation to trip during checking, but it never did") } @@ -154,11 +148,9 @@ func TestTscCancellationAborts(t *testing.T) { } } -// runWithCancellation runs the command line under a canceled context and returns the -// result. When midCheck is false the context is canceled before the run starts; when -// true it is canceled after checking has begun, and the returned bool reports whether -// that mid-check cancellation actually tripped. The run is guarded by a timeout so a -// regression that ignores cancellation fails loudly instead of hanging. +// runWithCancellation runs the command line under a context canceled either before +// the run starts (midCheck false) or after checking has begun (midCheck true). The +// returned bool reports whether a mid-check cancellation actually tripped. func runWithCancellation(t *testing.T, sys *TestSys, args []string, midCheck bool) (tsc.CommandLineResult, bool) { t.Helper() var ( @@ -166,8 +158,8 @@ func runWithCancellation(t *testing.T, sys *TestSys, args []string, midCheck boo midChecked func() bool ) if midCheck { - // Threshold small relative to the statement count so cancellation lands well - // before checking finishes. + // Small relative to the statement count, so cancellation lands well before + // checking finishes. c := newCancelAfterNPolls(5) ctx, midChecked = c, c.tripped.Load } else { @@ -189,9 +181,8 @@ func runWithCancellation(t *testing.T, sys *TestSys, args []string, midCheck boo } } -// runPreCanceled runs the command line on an existing sys under a context that is -// already canceled before the run starts, guarded by a timeout so a regression that -// ignores cancellation fails loudly instead of hanging. +// runPreCanceled runs the command line on an existing sys under an already-canceled +// context. func runPreCanceled(t *testing.T, sys *TestSys, args []string) tsc.CommandLineResult { t.Helper() ctx, cancel := context.WithCancel(context.Background()) @@ -209,10 +200,9 @@ func runPreCanceled(t *testing.T, sys *TestSys, args []string) tsc.CommandLineRe } } -// TestTscBuildCancellationUpToDate verifies that an interrupt is honored even when a -// `tsc -b` build has nothing to do. A root project that is already up to date has no -// upstream to wait on, so the no-build path must still observe a pre-canceled context -// and report ExitStatusCanceled rather than swallowing the interrupt as success. +// TestTscBuildCancellationUpToDate verifies that `tsc -b` honors an interrupt even +// with nothing to build. An up-to-date root has no upstream to wait on, so the +// no-build path is where the interrupt would otherwise be swallowed as success. func TestTscBuildCancellationUpToDate(t *testing.T) { t.Parallel() files := FileMap{ @@ -229,13 +219,61 @@ func TestTscBuildCancellationUpToDate(t *testing.T) { t.Fatalf("initial build status = %v, want ExitStatusSuccess", result.Status) } - // A second, pre-canceled build has nothing to build; it must still report canceled. result := runPreCanceled(t, sys, []string{"-b"}) if result.Status != tsc.ExitStatusCanceled { t.Errorf("status = %v, want ExitStatusCanceled", result.Status) } } +// TestTscBuildCancellationDuringGraph verifies that an interrupt during graph +// construction stops resolving configs and reports ExitStatusCanceled, rather than +// building from the partial graph or panicking on a task that was never created. +func TestTscBuildCancellationDuringGraph(t *testing.T) { + t.Parallel() + + // Each project references the next, so graph construction has several levels to + // resolve before it completes. + files := FileMap{} + const projects = 8 + for i := range projects { + dir := fmt.Sprintf("/home/src/workspaces/project/p%d", i) + references := "" + if i+1 < projects { + references = fmt.Sprintf(`, "references": [{ "path": "../p%d" }]`, i+1) + } + files[dir+"/tsconfig.json"] = fmt.Sprintf( + `{ "compilerOptions": { "composite": true }%s }`, references) + files[dir+fmt.Sprintf("/p%d.ts", i)] = "export {}\n" + } + + args := []string{"-b", "/home/src/workspaces/project/p0"} + + // Sweep rather than fix one threshold, so the graph phase keeps being exercised + // as the number of polls before it shifts. + for threshold := int32(0); threshold <= 20; threshold++ { + sys := newTestSys(&tscInput{commandLineArgs: args, files: files}, false) + ctx := newCancelAfterNPolls(threshold) + + var result tsc.CommandLineResult + func() { + defer func() { + if r := recover(); r != nil { + t.Fatalf("threshold=%d: panicked (want clean abort): %v", threshold, r) + } + }() + result = execute.CommandLine(ctx, sys, args, sys) + }() + + if ctx.tripped.Load() { + if result.Status != tsc.ExitStatusCanceled { + t.Fatalf("threshold=%d: status = %v, want ExitStatusCanceled", threshold, result.Status) + } + } else if result.Status != tsc.ExitStatusSuccess { + t.Fatalf("threshold=%d: status = %v, want ExitStatusSuccess (cancellation never tripped)", threshold, result.Status) + } + } +} + // TestTscCleanCancellation verifies that `tsc -b --clean` interrupted before it runs // does not delete outputs and reports ExitStatusCanceled instead of success. func TestTscCleanCancellation(t *testing.T) { @@ -250,7 +288,7 @@ func TestTscCleanCancellation(t *testing.T) { files: files, }, false) - // Build first so there is an output for clean to (potentially) delete. + // Build first so there is an output for clean to delete. if result := execute.CommandLine(context.Background(), sys, []string{"-b"}, sys); result.Status != tsc.ExitStatusSuccess { t.Fatalf("initial build status = %v, want ExitStatusSuccess", result.Status) } @@ -258,7 +296,6 @@ func TestTscCleanCancellation(t *testing.T) { t.Fatalf("expected %s to exist after build", outFile) } - // A pre-canceled clean must abort before deleting outputs and report canceled. result := runPreCanceled(t, sys, []string{"-b", "--clean"}) if result.Status != tsc.ExitStatusCanceled { t.Errorf("status = %v, want ExitStatusCanceled", result.Status) @@ -268,12 +305,11 @@ func TestTscCleanCancellation(t *testing.T) { } } -// TestTscCancellationSweep steps the cancellation point across the whole compile by -// increasing the poll threshold one step at a time, walking past the check phase and -// into emit. This covers windows a single fixed threshold would miss -- the -// no-emit-on-error recheck that runs during emit, the emit-returns-nil path, and -// declaration-emit type serialization -- and asserts that at no point does the run -// panic or report a partial result as success. +// TestTscCancellationSweep steps the cancellation point across the whole compile, +// past checking and into emit, asserting the run never panics or reports a partial +// result as success. A single fixed threshold would miss the narrow windows: the +// no-emit-on-error recheck, the emit-returns-nil path, and declaration-emit type +// serialization. func TestTscCancellationSweep(t *testing.T) { t.Parallel() @@ -303,9 +339,8 @@ func TestTscCancellationSweep(t *testing.T) { "/home/src/workspaces/project/b.ts": "export const b = 2;\nexport function g() { return [1, 2, 3] as const; }\n", } - // Upper bound comfortably exceeds a full clean run's poll count for this - // project, so the sweep covers check, the second global-diagnostics pass, - // and emit. + // The bound exceeds a full run's poll count for this project, so the sweep + // reaches check, the second global-diagnostics pass, and emit. for threshold := int32(1); threshold <= 150; threshold++ { sys := newTestSys(&tscInput{ commandLineArgs: []string{"--singleThreaded"}, @@ -323,9 +358,8 @@ func TestTscCancellationSweep(t *testing.T) { result = execute.CommandLine(ctx, sys, []string{"--singleThreaded"}, sys) }() - // Success only if cancellation never tripped (the build outran it); - // otherwise it must be a clean Canceled. Anything else means partial - // state leaked out. + // Success is only legitimate if the build outran the cancellation; + // anything else means partial state leaked out. if ctx.tripped.Load() { if result.Status != tsc.ExitStatusCanceled { t.Fatalf("threshold=%d: status = %v, want ExitStatusCanceled", threshold, result.Status) diff --git a/internal/execute/tsctests/watcher_race_test.go b/internal/execute/tsctests/watcher_race_test.go index 280c298c1fe..daa31566ea5 100644 --- a/internal/execute/tsctests/watcher_race_test.go +++ b/internal/execute/tsctests/watcher_race_test.go @@ -286,9 +286,8 @@ func TestBuildWatchStopsWhenContextIsCancelled(t *testing.T) { select { case result := <-resultCh: - // Cancellation is honored during the initial build: it aborts promptly without - // establishing a watcher. But cancellation is the expected way to end a watch, - // so it still reports success -- Ctrl-C is not a build failure. + // Success, because cancellation is the expected way to end a watch; no watcher, + // because the initial build aborted before establishing one. assert.Equal(t, result.Status, tsc.ExitStatusSuccess) assert.Assert(t, result.Watcher == nil) case <-time.After(2 * time.Second): From b6e1c50b120d12a28e3b7b46963e292989dd7386 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Tue, 11 Aug 2026 19:49:28 -0700 Subject: [PATCH 14/14] Pass context to EmitFilesAndReportErrors in emit_test emit_test.go arrived on main while this branch added a ctx parameter to EmitFilesAndReportErrors, so the merged tree failed to compile. Co-Authored-By: Claude Opus 5 (1M context) --- internal/execute/tsc/emit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/execute/tsc/emit_test.go b/internal/execute/tsc/emit_test.go index f2ac8b50491..34c4a5dfef5 100644 --- a/internal/execute/tsc/emit_test.go +++ b/internal/execute/tsc/emit_test.go @@ -132,7 +132,7 @@ export const make = (): Box => ({ value: "ok" }); } incrementalProgram := incremental.NewProgram(program, oldProgram, incremental.CreateHost(host), clock.NestedEmitNow, false) times := &CompileTimes{} - EmitFilesAndReportErrors(EmitInput{ + EmitFilesAndReportErrors(t.Context(), EmitInput{ Sys: sys, ProgramLike: incrementalProgram, Program: program,