OutputMonitor.continueMonitoringAsync starts a new monitoring run without stopping the previous one:
continueMonitoringAsync(token: CancellationToken): void {
this._asyncMode = true;
// Cancel and dispose any in-progress monitoring run to avoid two concurrent loops
this._currentMonitoringCts?.dispose();
this._currentMonitoringCts = new CancellationTokenSource(token);
this._state = OutputMonitorState.PollingForIdle;
this._startMonitoring(this._command, this._invocationContext, this._currentMonitoringCts.token);
}The inline comment says "Cancel and dispose any in-progress monitoring run", but the previous run isn't actually cancelled. CancellationTokenSource.dispose() at cancellation.ts#L128 defaults cancel to false:
dispose(cancel: boolean = false): void {
if (cancel) {
this.cancel();
}
...
}So disposing _currentMonitoringCts doesn't signal cancellation to the in-flight _startMonitoring call. The old run's token stays in its original state and the loop keeps progressing through _waitForIdle, _handleIdleState, and its finally block.
The foreground→background handoff in runInTerminalTool calls continueMonitoringAsync while the original monitoring pass is still running. With the cancellation bug above, that path can have two monitoring runs alive at once. OutputMonitor keeps per-run state on the instance, so both runs share this._state, this._pollingResult, this._userInputListener, and the emitters.
The stale-run finally at outputMonitor.ts#L187 is the strongest correctness problem:
} finally {
this._pollingResult = {
state: this._state,
output: output ?? this._execution.getOutput(),
pollDurationMs: Date.now() - pollStartTime,
resources
};
this._userInputListener.clear();
this._onDidFinishCommand.fire();
}There's no ownership check against the active monitoring run. A stale run can overwrite _pollingResult, clear _userInputListener, and fire onDidFinishCommand after a newer run has already taken over. If it reaches one of the input-required branches at outputMonitor.ts#L273, #L290, or #L302 before unwinding, it can also fire _onDidDetectInputNeeded against the new run's state.
Scoping: the initial async-mode flow from the constructor isn't affected because the caller waits for the first monitoring pass to finish before continueMonitoringAsync runs. This is specifically about the foreground→background transition path in runInTerminalTool.
OutputMonitor.continueMonitoringAsyncstarts a new monitoring run without stopping the previous one:The inline comment says "Cancel and dispose any in-progress monitoring run", but the previous run isn't actually cancelled.
CancellationTokenSource.dispose()at cancellation.ts#L128 defaultscanceltofalse:So disposing
_currentMonitoringCtsdoesn't signal cancellation to the in-flight_startMonitoringcall. The old run's token stays in its original state and the loop keeps progressing through_waitForIdle,_handleIdleState, and itsfinallyblock.The foreground→background handoff in
runInTerminalToolcallscontinueMonitoringAsyncwhile the original monitoring pass is still running. With the cancellation bug above, that path can have two monitoring runs alive at once.OutputMonitorkeeps per-run state on the instance, so both runs sharethis._state,this._pollingResult,this._userInputListener, and the emitters.The stale-run
finallyat outputMonitor.ts#L187 is the strongest correctness problem:There's no ownership check against the active monitoring run. A stale run can overwrite
_pollingResult, clear_userInputListener, and fireonDidFinishCommandafter a newer run has already taken over. If it reaches one of the input-required branches at outputMonitor.ts#L273, #L290, or #L302 before unwinding, it can also fire_onDidDetectInputNeededagainst the new run's state.Scoping: the initial async-mode flow from the constructor isn't affected because the caller waits for the first monitoring pass to finish before
continueMonitoringAsyncruns. This is specifically about the foreground→background transition path inrunInTerminalTool.