From 17a927c9366109d0be6dfe5cbbd5ef571765a6bb Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Wed, 22 Jul 2026 22:30:10 +0000 Subject: [PATCH 1/5] Fix extension host task system leak --- src/vs/workbench/api/browser/mainThreadTask.ts | 4 ++-- .../contrib/tasks/browser/abstractTaskService.ts | 16 +++++++++++++++- .../contrib/tasks/common/taskService.ts | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTask.ts b/src/vs/workbench/api/browser/mainThreadTask.ts index e151f5e4c2b31c..f80b127889692e 100644 --- a/src/vs/workbench/api/browser/mainThreadTask.ts +++ b/src/vs/workbench/api/browser/mainThreadTask.ts @@ -732,7 +732,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { default: platform = Platform.platform; } - this._taskService.registerTaskSystem(key, { + this._register(this._taskService.registerTaskSystem(key, { platform: platform, uriProvider: (path: string): URI => { return URI.from({ scheme: info.scheme, authority: info.authority, path }); @@ -777,7 +777,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { findExecutable: (command: string, cwd?: string, paths?: string[]): Promise => { return this._proxy.$findExecutable(command, cwd, paths); } - }); + })); } async $registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): Promise { diff --git a/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts b/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts index 18311c1cd1da54..c104627a34b3d8 100644 --- a/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts +++ b/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts @@ -874,7 +874,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer return infosCount > 0; } - public registerTaskSystem(key: string, info: ITaskSystemInfo): void { + public registerTaskSystem(key: string, info: ITaskSystemInfo): IDisposable { // Ideally the Web caller of registerRegisterTaskSystem would use the correct key. // However, the caller doesn't know about the workspace folders at the time of the call, even though we know about them here. if (info.platform === Platform.Platform.Web) { @@ -895,6 +895,20 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer if (this.hasTaskSystemInfo) { this._onDidChangeTaskSystemInfo.fire(); } + + return toDisposable(() => { + const infos = this._taskSystemInfos.get(key); + if (!infos) { + return; + } + const index = infos.indexOf(info); + if (index !== -1) { + infos.splice(index, 1); + } + if (infos.length === 0) { + this._taskSystemInfos.delete(key); + } + }); } private _getTaskSystemInfo(key: string): ITaskSystemInfo | undefined { diff --git a/src/vs/workbench/contrib/tasks/common/taskService.ts b/src/vs/workbench/contrib/tasks/common/taskService.ts index ec8414ee339f40..e665a8da0ccb14 100644 --- a/src/vs/workbench/contrib/tasks/common/taskService.ts +++ b/src/vs/workbench/contrib/tasks/common/taskService.ts @@ -104,7 +104,7 @@ export interface ITaskService { registerTaskProvider(taskProvider: ITaskProvider, type: string): IDisposable; - registerTaskSystem(scheme: string, taskSystemInfo: ITaskSystemInfo): void; + registerTaskSystem(scheme: string, taskSystemInfo: ITaskSystemInfo): IDisposable; readonly onDidChangeTaskSystemInfo: Event; readonly onDidChangeTaskConfig: Event; readonly hasTaskSystemInfo: boolean; From ccf5c6a18cb3dcc44d8a908fae04abffb6497e7e Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Mon, 17 Aug 2026 08:55:00 +0000 Subject: [PATCH 2/5] test: verify task system registrations are disposed --- .../api/test/browser/mainThreadTask.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/vs/workbench/api/test/browser/mainThreadTask.test.ts diff --git a/src/vs/workbench/api/test/browser/mainThreadTask.test.ts b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts new file mode 100644 index 00000000000000..e895e2a1108ba2 --- /dev/null +++ b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts @@ -0,0 +1,38 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Event } from '../../../../base/common/event.js'; +import { IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; +import { mock } from '../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { ITaskService } from '../../../contrib/tasks/common/taskService.js'; +import { MainThreadTask } from '../../browser/mainThreadTask.js'; +import { SingleProxyRPCProtocol } from '../common/testRPCProtocol.js'; + +suite('MainThreadTask', function () { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('unregisters task systems on dispose', function () { + let registrations = 0; + let disposals = 0; + const taskService = new class extends mock() { + override readonly onDidStateChange = Event.None; + + override registerTaskSystem(): IDisposable { + registrations++; + return toDisposable(() => disposals++); + } + }; + const mainThreadTask = new MainThreadTask(SingleProxyRPCProtocol(null), taskService, undefined!, undefined!); + + mainThreadTask.$registerTaskSystem('file', { scheme: 'file', authority: '', platform: 'linux' }); + assert.strictEqual(registrations, 1); + + mainThreadTask.dispose(); + assert.strictEqual(disposals, 1); + }); +}); From f42c574d8afdd53d0e6ec8f7db34b3eeb6739ca6 Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Mon, 17 Aug 2026 12:02:38 +0000 Subject: [PATCH 3/5] Refactor task system registration lifecycle --- .../workbench/api/browser/mainThreadTask.ts | 18 +++++++-- .../workbench/api/common/extHost.protocol.ts | 3 +- src/vs/workbench/api/common/extHostTask.ts | 16 +++++--- src/vs/workbench/api/node/extHostTask.ts | 8 ++-- .../api/test/browser/extHostTask.test.ts | 37 +++++++++++++++++++ .../api/test/browser/mainThreadTask.test.ts | 16 ++++---- 6 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 src/vs/workbench/api/test/browser/extHostTask.test.ts diff --git a/src/vs/workbench/api/browser/mainThreadTask.ts b/src/vs/workbench/api/browser/mainThreadTask.ts index f80b127889692e..dadb36f9ced1c3 100644 --- a/src/vs/workbench/api/browser/mainThreadTask.ts +++ b/src/vs/workbench/api/browser/mainThreadTask.ts @@ -458,6 +458,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { private readonly _extHostContext: IExtHostContext | undefined; private readonly _proxy: ExtHostTaskShape; private readonly _providers: Map; + private readonly _taskSystems: Map; constructor( extHostContext: IExtHostContext, @@ -468,6 +469,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { super(); this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostTask); this._providers = new Map(); + this._taskSystems = new Map(); this._register(this._taskService.onDidStateChange(async (event: ITaskEvent) => { if (event.kind === TaskEventKind.Changed) { return; @@ -511,6 +513,10 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { value.disposable.dispose(); } this._providers.clear(); + for (const disposable of this._taskSystems.values()) { + disposable.dispose(); + } + this._taskSystems.clear(); super.dispose(); } @@ -714,7 +720,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { }); } - public $registerTaskSystem(key: string, info: ITaskSystemInfoDTO): void { + public $registerTaskSystem(handle: number, key: string, info: ITaskSystemInfoDTO): void { let platform: Platform.Platform; switch (info.platform) { case 'Web': @@ -732,7 +738,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { default: platform = Platform.platform; } - this._register(this._taskService.registerTaskSystem(key, { + const disposable = this._taskService.registerTaskSystem(key, { platform: platform, uriProvider: (path: string): URI => { return URI.from({ scheme: info.scheme, authority: info.authority, path }); @@ -777,7 +783,13 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { findExecutable: (command: string, cwd?: string, paths?: string[]): Promise => { return this._proxy.$findExecutable(command, cwd, paths); } - })); + }); + this._taskSystems.set(handle, disposable); + } + + public $unregisterTaskSystem(handle: number): void { + this._taskSystems.get(handle)?.dispose(); + this._taskSystems.delete(handle); } async $registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): Promise { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c0828beefe03a2..d33aa18b65b88c 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2048,7 +2048,8 @@ export interface MainThreadTaskShape extends IDisposable { $getTaskExecution(value: tasks.ITaskHandleDTO | tasks.ITaskDTO): Promise; $executeTask(task: tasks.ITaskHandleDTO | tasks.ITaskDTO): Promise; $terminateTask(id: string): Promise; - $registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void; + $registerTaskSystem(handle: number, scheme: string, info: tasks.ITaskSystemInfoDTO): void; + $unregisterTaskSystem(handle: number): void; $customExecutionComplete(id: string, result?: number): Promise; $registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): Promise; } diff --git a/src/vs/workbench/api/common/extHostTask.ts b/src/vs/workbench/api/common/extHostTask.ts index 7191d7b5ac35d0..e556efc67feffd 100644 --- a/src/vs/workbench/api/common/extHostTask.ts +++ b/src/vs/workbench/api/common/extHostTask.ts @@ -28,6 +28,7 @@ import { USER_TASKS_GROUP_KEY } from '../../contrib/tasks/common/tasks.js'; import { ErrorNoTelemetry, NotSupportedError } from '../../../base/common/errors.js'; import { asArray } from '../../../base/common/arrays.js'; import { ITaskProblemMatcherStartedDto, ITaskProblemMatcherEndedDto } from './shared/tasks.js'; +import { Disposable } from '../../../base/common/lifecycle.js'; export interface IExtHostTask extends ExtHostTaskShape { @@ -42,7 +43,7 @@ export interface IExtHostTask extends ExtHostTaskShape { readonly onDidEndTaskProblemMatchers: Event; registerTaskProvider(extension: IExtensionDescription, type: string, provider: vscode.TaskProvider): vscode.Disposable; - registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void; + registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): vscode.Disposable; fetchTasks(filter?: vscode.TaskFilter): Promise; executeTask(extension: IExtensionDescription, task: vscode.Task): Promise; terminateTask(execution: vscode.TaskExecution): Promise; @@ -396,7 +397,7 @@ export interface HandlerData { extension: IExtensionDescription; } -export abstract class ExtHostTaskBase implements ExtHostTaskShape, IExtHostTask { +export abstract class ExtHostTaskBase extends Disposable implements ExtHostTaskShape, IExtHostTask { readonly _serviceBrand: undefined; protected readonly _proxy: MainThreadTaskShape; @@ -432,6 +433,7 @@ export abstract class ExtHostTaskBase implements ExtHostTaskShape, IExtHostTask @ILogService logService: ILogService, @IExtHostApiDeprecationService deprecationService: IExtHostApiDeprecationService ) { + super(); this._proxy = extHostRpc.getProxy(MainContext.MainThreadTask); this._workspaceProvider = workspaceService; this._editorService = editorService; @@ -462,8 +464,10 @@ export abstract class ExtHostTaskBase implements ExtHostTaskShape, IExtHostTask }); } - public registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void { - this._proxy.$registerTaskSystem(scheme, info); + public registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): vscode.Disposable { + const handle = this.nextHandle(); + this._proxy.$registerTaskSystem(handle, scheme, info); + return new types.Disposable(() => this._proxy.$unregisterTaskSystem(handle)); } public fetchTasks(filter?: vscode.TaskFilter): Promise { @@ -760,11 +764,11 @@ export class WorkerExtHostTask extends ExtHostTaskBase { @IExtHostApiDeprecationService deprecationService: IExtHostApiDeprecationService ) { super(extHostRpc, initData, workspaceService, editorService, configurationService, extHostTerminalService, logService, deprecationService); - this.registerTaskSystem(Schemas.vscodeRemote, { + this._register(this.registerTaskSystem(Schemas.vscodeRemote, { scheme: Schemas.vscodeRemote, authority: '', platform: Platform.PlatformToString(Platform.Platform.Web) - }); + })); } public async executeTask(extension: IExtensionDescription, task: vscode.Task): Promise { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index b6cfc881ec4e8a..c8682a13aeee28 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -40,17 +40,17 @@ export class ExtHostTask extends ExtHostTaskBase { ) { super(extHostRpc, initData, workspaceService, editorService, configurationService, extHostTerminalService, logService, deprecationService); if (initData.remote.isRemote && initData.remote.authority) { - this.registerTaskSystem(Schemas.vscodeRemote, { + this._register(this.registerTaskSystem(Schemas.vscodeRemote, { scheme: Schemas.vscodeRemote, authority: initData.remote.authority, platform: process.platform - }); + })); } else { - this.registerTaskSystem(Schemas.file, { + this._register(this.registerTaskSystem(Schemas.file, { scheme: Schemas.file, authority: '', platform: process.platform - }); + })); } this._proxy.$registerSupportedExecutions(true, true, true); } diff --git a/src/vs/workbench/api/test/browser/extHostTask.test.ts b/src/vs/workbench/api/test/browser/extHostTask.test.ts new file mode 100644 index 00000000000000..5e425016b96697 --- /dev/null +++ b/src/vs/workbench/api/test/browser/extHostTask.test.ts @@ -0,0 +1,37 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { mock } from '../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { MainThreadTaskShape } from '../../common/extHost.protocol.js'; +import { WorkerExtHostTask } from '../../common/extHostTask.js'; +import { SingleProxyRPCProtocol } from '../common/testRPCProtocol.js'; + +suite('ExtHostTask', function () { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('unregisters task systems on dispose', function () { + const events: string[] = []; + const proxy = new class extends mock() { + override $registerTaskSystem(handle: number): void { + events.push(`register ${handle}`); + } + + override $unregisterTaskSystem(handle: number): void { + events.push(`unregister ${handle}`); + } + + override $registerSupportedExecutions(): Promise { + return Promise.resolve(); + } + }; + const extHostTask = new WorkerExtHostTask(SingleProxyRPCProtocol(proxy), undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!); + + extHostTask.dispose(); + assert.deepStrictEqual(events, ['register 0', 'unregister 0']); + }); +}); diff --git a/src/vs/workbench/api/test/browser/mainThreadTask.test.ts b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts index e895e2a1108ba2..1e048103c5f94c 100644 --- a/src/vs/workbench/api/test/browser/mainThreadTask.test.ts +++ b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts @@ -16,23 +16,25 @@ suite('MainThreadTask', function () { ensureNoDisposablesAreLeakedInTestSuite(); - test('unregisters task systems on dispose', function () { + test('unregisters task systems explicitly and on dispose', function () { + const events: string[] = []; let registrations = 0; - let disposals = 0; const taskService = new class extends mock() { override readonly onDidStateChange = Event.None; override registerTaskSystem(): IDisposable { - registrations++; - return toDisposable(() => disposals++); + const registration = ++registrations; + events.push(`register ${registration}`); + return toDisposable(() => events.push(`dispose ${registration}`)); } }; const mainThreadTask = new MainThreadTask(SingleProxyRPCProtocol(null), taskService, undefined!, undefined!); - mainThreadTask.$registerTaskSystem('file', { scheme: 'file', authority: '', platform: 'linux' }); - assert.strictEqual(registrations, 1); + mainThreadTask.$registerTaskSystem(0, 'file', { scheme: 'file', authority: '', platform: 'linux' }); + mainThreadTask.$unregisterTaskSystem(0); + mainThreadTask.$registerTaskSystem(1, 'file', { scheme: 'file', authority: '', platform: 'linux' }); mainThreadTask.dispose(); - assert.strictEqual(disposals, 1); + assert.deepStrictEqual(events, ['register 1', 'dispose 1', 'register 2', 'dispose 2']); }); }); From 2e4052d7c7a97b78de1ccdf0371ab178d0fc3961 Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Mon, 17 Aug 2026 12:33:21 +0000 Subject: [PATCH 4/5] Simplify task system registration lifecycle --- .../workbench/api/browser/mainThreadTask.ts | 18 ++------- .../workbench/api/common/extHost.protocol.ts | 3 +- src/vs/workbench/api/common/extHostTask.ts | 16 +++----- src/vs/workbench/api/node/extHostTask.ts | 8 ++-- .../api/test/browser/extHostTask.test.ts | 37 ------------------- .../api/test/browser/mainThreadTask.test.ts | 16 ++++---- 6 files changed, 21 insertions(+), 77 deletions(-) delete mode 100644 src/vs/workbench/api/test/browser/extHostTask.test.ts diff --git a/src/vs/workbench/api/browser/mainThreadTask.ts b/src/vs/workbench/api/browser/mainThreadTask.ts index dadb36f9ced1c3..f80b127889692e 100644 --- a/src/vs/workbench/api/browser/mainThreadTask.ts +++ b/src/vs/workbench/api/browser/mainThreadTask.ts @@ -458,7 +458,6 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { private readonly _extHostContext: IExtHostContext | undefined; private readonly _proxy: ExtHostTaskShape; private readonly _providers: Map; - private readonly _taskSystems: Map; constructor( extHostContext: IExtHostContext, @@ -469,7 +468,6 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { super(); this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostTask); this._providers = new Map(); - this._taskSystems = new Map(); this._register(this._taskService.onDidStateChange(async (event: ITaskEvent) => { if (event.kind === TaskEventKind.Changed) { return; @@ -513,10 +511,6 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { value.disposable.dispose(); } this._providers.clear(); - for (const disposable of this._taskSystems.values()) { - disposable.dispose(); - } - this._taskSystems.clear(); super.dispose(); } @@ -720,7 +714,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { }); } - public $registerTaskSystem(handle: number, key: string, info: ITaskSystemInfoDTO): void { + public $registerTaskSystem(key: string, info: ITaskSystemInfoDTO): void { let platform: Platform.Platform; switch (info.platform) { case 'Web': @@ -738,7 +732,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { default: platform = Platform.platform; } - const disposable = this._taskService.registerTaskSystem(key, { + this._register(this._taskService.registerTaskSystem(key, { platform: platform, uriProvider: (path: string): URI => { return URI.from({ scheme: info.scheme, authority: info.authority, path }); @@ -783,13 +777,7 @@ export class MainThreadTask extends Disposable implements MainThreadTaskShape { findExecutable: (command: string, cwd?: string, paths?: string[]): Promise => { return this._proxy.$findExecutable(command, cwd, paths); } - }); - this._taskSystems.set(handle, disposable); - } - - public $unregisterTaskSystem(handle: number): void { - this._taskSystems.get(handle)?.dispose(); - this._taskSystems.delete(handle); + })); } async $registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): Promise { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index d33aa18b65b88c..c0828beefe03a2 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2048,8 +2048,7 @@ export interface MainThreadTaskShape extends IDisposable { $getTaskExecution(value: tasks.ITaskHandleDTO | tasks.ITaskDTO): Promise; $executeTask(task: tasks.ITaskHandleDTO | tasks.ITaskDTO): Promise; $terminateTask(id: string): Promise; - $registerTaskSystem(handle: number, scheme: string, info: tasks.ITaskSystemInfoDTO): void; - $unregisterTaskSystem(handle: number): void; + $registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void; $customExecutionComplete(id: string, result?: number): Promise; $registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): Promise; } diff --git a/src/vs/workbench/api/common/extHostTask.ts b/src/vs/workbench/api/common/extHostTask.ts index e556efc67feffd..7191d7b5ac35d0 100644 --- a/src/vs/workbench/api/common/extHostTask.ts +++ b/src/vs/workbench/api/common/extHostTask.ts @@ -28,7 +28,6 @@ import { USER_TASKS_GROUP_KEY } from '../../contrib/tasks/common/tasks.js'; import { ErrorNoTelemetry, NotSupportedError } from '../../../base/common/errors.js'; import { asArray } from '../../../base/common/arrays.js'; import { ITaskProblemMatcherStartedDto, ITaskProblemMatcherEndedDto } from './shared/tasks.js'; -import { Disposable } from '../../../base/common/lifecycle.js'; export interface IExtHostTask extends ExtHostTaskShape { @@ -43,7 +42,7 @@ export interface IExtHostTask extends ExtHostTaskShape { readonly onDidEndTaskProblemMatchers: Event; registerTaskProvider(extension: IExtensionDescription, type: string, provider: vscode.TaskProvider): vscode.Disposable; - registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): vscode.Disposable; + registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void; fetchTasks(filter?: vscode.TaskFilter): Promise; executeTask(extension: IExtensionDescription, task: vscode.Task): Promise; terminateTask(execution: vscode.TaskExecution): Promise; @@ -397,7 +396,7 @@ export interface HandlerData { extension: IExtensionDescription; } -export abstract class ExtHostTaskBase extends Disposable implements ExtHostTaskShape, IExtHostTask { +export abstract class ExtHostTaskBase implements ExtHostTaskShape, IExtHostTask { readonly _serviceBrand: undefined; protected readonly _proxy: MainThreadTaskShape; @@ -433,7 +432,6 @@ export abstract class ExtHostTaskBase extends Disposable implements ExtHostTaskS @ILogService logService: ILogService, @IExtHostApiDeprecationService deprecationService: IExtHostApiDeprecationService ) { - super(); this._proxy = extHostRpc.getProxy(MainContext.MainThreadTask); this._workspaceProvider = workspaceService; this._editorService = editorService; @@ -464,10 +462,8 @@ export abstract class ExtHostTaskBase extends Disposable implements ExtHostTaskS }); } - public registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): vscode.Disposable { - const handle = this.nextHandle(); - this._proxy.$registerTaskSystem(handle, scheme, info); - return new types.Disposable(() => this._proxy.$unregisterTaskSystem(handle)); + public registerTaskSystem(scheme: string, info: tasks.ITaskSystemInfoDTO): void { + this._proxy.$registerTaskSystem(scheme, info); } public fetchTasks(filter?: vscode.TaskFilter): Promise { @@ -764,11 +760,11 @@ export class WorkerExtHostTask extends ExtHostTaskBase { @IExtHostApiDeprecationService deprecationService: IExtHostApiDeprecationService ) { super(extHostRpc, initData, workspaceService, editorService, configurationService, extHostTerminalService, logService, deprecationService); - this._register(this.registerTaskSystem(Schemas.vscodeRemote, { + this.registerTaskSystem(Schemas.vscodeRemote, { scheme: Schemas.vscodeRemote, authority: '', platform: Platform.PlatformToString(Platform.Platform.Web) - })); + }); } public async executeTask(extension: IExtensionDescription, task: vscode.Task): Promise { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index c8682a13aeee28..b6cfc881ec4e8a 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -40,17 +40,17 @@ export class ExtHostTask extends ExtHostTaskBase { ) { super(extHostRpc, initData, workspaceService, editorService, configurationService, extHostTerminalService, logService, deprecationService); if (initData.remote.isRemote && initData.remote.authority) { - this._register(this.registerTaskSystem(Schemas.vscodeRemote, { + this.registerTaskSystem(Schemas.vscodeRemote, { scheme: Schemas.vscodeRemote, authority: initData.remote.authority, platform: process.platform - })); + }); } else { - this._register(this.registerTaskSystem(Schemas.file, { + this.registerTaskSystem(Schemas.file, { scheme: Schemas.file, authority: '', platform: process.platform - })); + }); } this._proxy.$registerSupportedExecutions(true, true, true); } diff --git a/src/vs/workbench/api/test/browser/extHostTask.test.ts b/src/vs/workbench/api/test/browser/extHostTask.test.ts deleted file mode 100644 index 5e425016b96697..00000000000000 --- a/src/vs/workbench/api/test/browser/extHostTask.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as assert from 'assert'; -import { mock } from '../../../../base/test/common/mock.js'; -import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { MainThreadTaskShape } from '../../common/extHost.protocol.js'; -import { WorkerExtHostTask } from '../../common/extHostTask.js'; -import { SingleProxyRPCProtocol } from '../common/testRPCProtocol.js'; - -suite('ExtHostTask', function () { - - ensureNoDisposablesAreLeakedInTestSuite(); - - test('unregisters task systems on dispose', function () { - const events: string[] = []; - const proxy = new class extends mock() { - override $registerTaskSystem(handle: number): void { - events.push(`register ${handle}`); - } - - override $unregisterTaskSystem(handle: number): void { - events.push(`unregister ${handle}`); - } - - override $registerSupportedExecutions(): Promise { - return Promise.resolve(); - } - }; - const extHostTask = new WorkerExtHostTask(SingleProxyRPCProtocol(proxy), undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!); - - extHostTask.dispose(); - assert.deepStrictEqual(events, ['register 0', 'unregister 0']); - }); -}); diff --git a/src/vs/workbench/api/test/browser/mainThreadTask.test.ts b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts index 1e048103c5f94c..e895e2a1108ba2 100644 --- a/src/vs/workbench/api/test/browser/mainThreadTask.test.ts +++ b/src/vs/workbench/api/test/browser/mainThreadTask.test.ts @@ -16,25 +16,23 @@ suite('MainThreadTask', function () { ensureNoDisposablesAreLeakedInTestSuite(); - test('unregisters task systems explicitly and on dispose', function () { - const events: string[] = []; + test('unregisters task systems on dispose', function () { let registrations = 0; + let disposals = 0; const taskService = new class extends mock() { override readonly onDidStateChange = Event.None; override registerTaskSystem(): IDisposable { - const registration = ++registrations; - events.push(`register ${registration}`); - return toDisposable(() => events.push(`dispose ${registration}`)); + registrations++; + return toDisposable(() => disposals++); } }; const mainThreadTask = new MainThreadTask(SingleProxyRPCProtocol(null), taskService, undefined!, undefined!); - mainThreadTask.$registerTaskSystem(0, 'file', { scheme: 'file', authority: '', platform: 'linux' }); - mainThreadTask.$unregisterTaskSystem(0); - mainThreadTask.$registerTaskSystem(1, 'file', { scheme: 'file', authority: '', platform: 'linux' }); + mainThreadTask.$registerTaskSystem('file', { scheme: 'file', authority: '', platform: 'linux' }); + assert.strictEqual(registrations, 1); mainThreadTask.dispose(); - assert.deepStrictEqual(events, ['register 1', 'dispose 1', 'register 2', 'dispose 2']); + assert.strictEqual(disposals, 1); }); }); From a4995de3988e12564919058dbfe79150bd4a4a28 Mon Sep 17 00:00:00 2001 From: Simon Siefke Date: Mon, 17 Aug 2026 12:50:53 +0000 Subject: [PATCH 5/5] Address task system disposal review feedback --- .../tasks/browser/abstractTaskService.ts | 8 ++-- .../test/browser/abstractTaskService.test.ts | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 src/vs/workbench/contrib/tasks/test/browser/abstractTaskService.test.ts diff --git a/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts b/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts index 6d5ca3394d3526..612a9db8fb593d 100644 --- a/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts +++ b/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts @@ -879,7 +879,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer } public registerTaskSystem(key: string, info: ITaskSystemInfo): IDisposable { - // Ideally the Web caller of registerRegisterTaskSystem would use the correct key. + // Ideally the Web caller of registerTaskSystem would use the correct key. // However, the caller doesn't know about the workspace folders at the time of the call, even though we know about them here. if (info.platform === Platform.Platform.Web) { key = this.workspaceFolders.length ? this.workspaceFolders[0].uri.scheme : key; @@ -906,12 +906,14 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer return; } const index = infos.indexOf(info); - if (index !== -1) { - infos.splice(index, 1); + if (index === -1) { + return; } + infos.splice(index, 1); if (infos.length === 0) { this._taskSystemInfos.delete(key); } + this._onDidChangeTaskSystemInfo.fire(); }); } diff --git a/src/vs/workbench/contrib/tasks/test/browser/abstractTaskService.test.ts b/src/vs/workbench/contrib/tasks/test/browser/abstractTaskService.test.ts new file mode 100644 index 00000000000000..47dc6a0a0a9679 --- /dev/null +++ b/src/vs/workbench/contrib/tasks/test/browser/abstractTaskService.test.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Emitter } from '../../../../../base/common/event.js'; +import * as Platform from '../../../../../base/common/platform.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { AbstractTaskService } from '../../browser/abstractTaskService.js'; +import { ITaskSystemInfo } from '../../common/taskSystem.js'; + +suite('AbstractTaskService', function () { + + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + test('removes task system info and emits a change when disposed', function () { + const taskSystemInfoEmitter = store.add(new Emitter()); + const taskService = Object.create(AbstractTaskService.prototype) as AbstractTaskService; + Reflect.set(taskService, '_taskSystemInfos', new Map()); + Reflect.set(taskService, '_environmentService', { remoteAuthority: undefined }); + Reflect.set(taskService, '_onDidChangeTaskSystemInfo', taskSystemInfoEmitter); + Reflect.set(taskService, 'onDidChangeTaskSystemInfo', taskSystemInfoEmitter.event); + + const states: boolean[] = []; + store.add(taskService.onDidChangeTaskSystemInfo(() => states.push(taskService.hasTaskSystemInfo))); + const registration = store.add(taskService.registerTaskSystem('file', { + platform: Platform.Platform.Linux, + context: undefined, + uriProvider: path => URI.file(path), + resolveVariables: async () => undefined, + findExecutable: async () => undefined + })); + + registration.dispose(); + + assert.deepStrictEqual(states, [true, false]); + }); +});