Skip to content

Commit 7d58583

Browse files
authored
Fix run_in_terminal wedging when cached terminal is disposed (#311019)
fix issues with disposed terminal
1 parent 32162cf commit 7d58583

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1839,7 +1839,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
18391839
// For foreground mode, try to reuse cached terminal (but not if it was a background terminal)
18401840
if (!isBackground) {
18411841
const cachedTerminal = this._sessionTerminalAssociations.get(chatSessionResource);
1842-
if (cachedTerminal && !cachedTerminal.isBackground) {
1842+
if (cachedTerminal && !cachedTerminal.isBackground && !cachedTerminal.instance.isDisposed) {
18431843
this._logService.debug(`RunInTerminalTool: Using cached terminal with session resource \`${chatSessionResource}\``);
18441844
this._terminalToolCreator.refreshShellIntegrationQuality(cachedTerminal);
18451845
this._terminalChatService.registerTerminalInstanceWithToolSession(terminalToolSessionId, cachedTerminal.instance);

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,6 +1900,30 @@ suite('RunInTerminalTool', () => {
19001900
chatServiceDisposeEmitter.fire({ sessionResources: [LocalChatSessionUri.forSession('non-existent-session')], reason: 'cleared' });
19011901
strictEqual(runInTerminalTool.sessionTerminalAssociations.size, 0, 'No associations should exist after handling non-existent session');
19021902
});
1903+
1904+
test('should not reuse a disposed cached terminal', () => {
1905+
const sessionResource = LocalChatSessionUri.forSession('disposed-terminal-session');
1906+
const disposedTerminal = {
1907+
isDisposed: true,
1908+
dispose: () => { },
1909+
processId: 99999,
1910+
} as unknown as ITerminalInstance;
1911+
runInTerminalTool.sessionTerminalAssociations.set(sessionResource, {
1912+
instance: disposedTerminal,
1913+
shellIntegrationQuality: ShellIntegrationQuality.None,
1914+
isBackground: false,
1915+
});
1916+
1917+
// A disposed cached terminal should not be returned by the association lookup
1918+
const cachedTerminal = runInTerminalTool.sessionTerminalAssociations.get(sessionResource);
1919+
ok(cachedTerminal, 'Cached terminal should exist in the map');
1920+
strictEqual(cachedTerminal!.instance.isDisposed, true, 'Cached terminal should be disposed');
1921+
1922+
// Verify the guard condition that _initTerminal uses:
1923+
// cachedTerminal && !cachedTerminal.isBackground && !cachedTerminal.instance.isDisposed
1924+
const wouldReuse = cachedTerminal !== undefined && !cachedTerminal.isBackground && !cachedTerminal.instance.isDisposed;
1925+
strictEqual(wouldReuse, false, 'Should not reuse a disposed cached terminal');
1926+
});
19031927
});
19041928

19051929
test('should dedupe rapid repeated background input-needed notifications', () => {

0 commit comments

Comments
 (0)