fix: memory leak in main thread terminal service (#286236)

* fix: memory leak in mainThreadTerminalService

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix formatting

* fix

* undo unrelated change

* address review: cleanup tuple return, unused import, heartbeat cleanup

* add ptyHostService restart listener leak test

* _disposePtyHost: avoid spawning pty host during shutdown

* Minor tweaks

* Minor tweaks

Co-authored-by: Copilot <copilot@github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Dmitriy Vasyura <dmitriv@microsoft.com>
Co-authored-by: Copilot <copilot@github.com>
This commit is contained in:
Simon Siefke
2026-06-04 11:09:05 +02:00
committed by GitHub
parent 6c8a54f8df
commit 0df366a8a9
@@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { DisposableStore, Disposable, IDisposable, MutableDisposable, combinedDisposable, toDisposable } from '../../../base/common/lifecycle.js';
import { DisposableStore, Disposable, IDisposable, MutableDisposable, combinedDisposable, DisposableMap } from '../../../base/common/lifecycle.js';
import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, TerminalLaunchConfig, ITerminalDimensionsDto, ExtHostTerminalIdentifier, TerminalQuickFix, ITerminalCommandDto } from '../common/extHost.protocol.js';
import { extHostNamedCustomer, IExtHostContext } from '../../services/extensions/common/extHostCustomers.js';
import { URI } from '../../../base/common/uri.js';
@@ -28,6 +28,10 @@ import { ITerminalCompletionService } from '../../contrib/terminalContrib/sugges
import { IWorkbenchEnvironmentService } from '../../services/environment/common/environmentService.js';
import { hasKey } from '../../../base/common/types.js';
interface TerminalProcessProxyEntry extends IDisposable {
readonly proxy: ITerminalProcessExtHostProxy;
}
@extHostNamedCustomer(MainContext.MainThreadTerminalService)
export class MainThreadTerminalService extends Disposable implements MainThreadTerminalServiceShape {
@@ -39,10 +43,10 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
* This comes in play only when dealing with terminals created on the extension host side
*/
private readonly _extHostTerminals = new Map<string, Promise<ITerminalInstance>>();
private readonly _terminalProcessProxies = new Map<number, { proxy: ITerminalProcessExtHostProxy; store: DisposableStore }>();
private readonly _profileProviders = new Map<string, IDisposable>();
private readonly _completionProviders = new Map<string, IDisposable>();
private readonly _quickFixProviders = new Map<string, IDisposable>();
private readonly _terminalProcessProxies = this._register(new DisposableMap<number, TerminalProcessProxyEntry>());
private readonly _profileProviders = this._register(new DisposableMap<string, IDisposable>());
private readonly _completionProviders = this._register(new DisposableMap<string, IDisposable>());
private readonly _quickFixProviders = this._register(new DisposableMap<string, IDisposable>());
private readonly _dataEventTracker = this._register(new MutableDisposable<TerminalDataEventTracker>());
private readonly _sendCommandEventListener = this._register(new MutableDisposable());
@@ -111,29 +115,11 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
});
this._proxy.$initEnvironmentVariableCollections(serializedCollections);
}
this._store.add(toDisposable(() => {
for (const e of this._terminalProcessProxies.values()) {
e.proxy.dispose();
e.store.dispose();
}
this._terminalProcessProxies.clear();
}));
remoteAgentService.getEnvironment().then(async env => {
this._os = env?.os || OS;
this._updateDefaultProfile();
});
this._register(this._terminalProfileService.onDidChangeAvailableProfiles(() => this._updateDefaultProfile()));
this._register(toDisposable(() => {
for (const provider of this._profileProviders.values()) {
provider.dispose();
}
for (const provider of this._quickFixProviders.values()) {
provider.dispose();
}
}));
}
private async _updateDefaultProfile() {
@@ -314,8 +300,7 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
}
public $unregisterCompletionProvider(id: string): void {
this._completionProviders.get(id)?.dispose();
this._completionProviders.delete(id);
this._completionProviders.deleteAndDispose(id);
}
public $registerProfileProvider(id: string, extensionIdentifier: string): void {
@@ -328,8 +313,7 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
}
public $unregisterProfileProvider(id: string): void {
this._profileProviders.get(id)?.dispose();
this._profileProviders.delete(id);
this._profileProviders.deleteAndDispose(id);
}
public async $registerQuickFixProvider(id: string, extensionId: string): Promise<void> {
@@ -370,8 +354,7 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
}
public $unregisterQuickFixProvider(id: string): void {
this._quickFixProviders.get(id)?.dispose();
this._quickFixProviders.delete(id);
this._quickFixProviders.deleteAndDispose(id);
}
private _onActiveTerminalChanged(terminalId: number | null): void {
@@ -399,12 +382,7 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
private _onTerminalDisposed(terminalInstance: ITerminalInstance): void {
this._proxy.$acceptTerminalClosed(terminalInstance.instanceId, terminalInstance.exitCode, terminalInstance.exitReason ?? TerminalExitReason.Unknown);
const proxy = this._terminalProcessProxies.get(terminalInstance.instanceId);
if (proxy) {
proxy.proxy.dispose();
proxy.store.dispose();
this._terminalProcessProxies.delete(terminalInstance.instanceId);
}
this._terminalProcessProxies.deleteAndDispose(terminalInstance.instanceId);
}
private _onTerminalOpened(terminalInstance: ITerminalInstance): void {
@@ -440,7 +418,8 @@ export class MainThreadTerminalService extends Disposable implements MainThreadT
private _onRequestStartExtensionTerminal(request: IStartExtensionTerminalRequest): void {
const proxy = request.proxy;
const store = new DisposableStore();
this._terminalProcessProxies.set(proxy.instanceId, { proxy, store });
store.add(proxy);
this._terminalProcessProxies.set(proxy.instanceId, { proxy, dispose: () => store.dispose() });
// Note that onResize is not being listened to here as it needs to fire when max dimensions
// change, excluding the dimension override