Skip to content

Commit a837f16

Browse files
alexdimaalexdima
authored
Report Event.Buffer leaks when running from source (#298468)
* Report Event.Buffer leaks when running from source * Address feedback
1 parent c60fe51 commit a837f16

File tree

7 files changed

+62
-19
lines changed

7 files changed

+62
-19
lines changed

‎src/vs/base/common/event.ts‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { createSingleCallFunction } from './functional.js';
1111
import { combinedDisposable, Disposable, DisposableMap, DisposableStore, IDisposable, toDisposable } from './lifecycle.js';
1212
import { LinkedList } from './linkedList.js';
1313
import { IObservable, IObservableWithChange, IObserver } from './observable.js';
14+
import { env } from './process.js';
1415
import { StopWatch } from './stopwatch.js';
1516
import { MicrotaskDelay } from './symbols.js';
1617

@@ -31,6 +32,14 @@ const _enableSnapshotPotentialLeakWarning = false
3132
// || Boolean("TRUE") // causes a linter warning so that it cannot be pushed
3233
;
3334

35+
36+
const _bufferLeakWarnCountThreshold = 100;
37+
const _bufferLeakWarnTimeThreshold = 60_000; // 1 minute
38+
39+
function _isBufferLeakWarningEnabled(): boolean {
40+
return !!env['VSCODE_DEV'];
41+
}
42+
3443
/**
3544
* An event with zero or one parameters that can be subscribed to. The event is a function itself.
3645
*/
@@ -490,6 +499,7 @@ export namespace Event {
490499
* returned event causes this utility to leak a listener on the original event.
491500
*
492501
* @param event The event source for the new event.
502+
* @param debugName A name for this buffer, used in leak detection warnings.
493503
* @param flushAfterTimeout Determines whether to flush the buffer after a timeout immediately or after a
494504
* `setTimeout` when the first event listener is added.
495505
* @param _buffer Internal: A source event array used for tests.
@@ -499,15 +509,46 @@ export namespace Event {
499509
* // Start accumulating events, when the first listener is attached, flush
500510
* // the event after a timeout such that multiple listeners attached before
501511
* // the timeout would receive the event
502-
* this.onInstallExtension = Event.buffer(service.onInstallExtension, true);
512+
* this.onInstallExtension = Event.buffer(service.onInstallExtension, 'onInstallExtension', true);
503513
* ```
504514
*/
505-
export function buffer<T>(event: Event<T>, flushAfterTimeout = false, _buffer: T[] = [], disposable?: DisposableStore): Event<T> {
515+
export function buffer<T>(event: Event<T>, debugName: string, flushAfterTimeout = false, _buffer: T[] = [], disposable?: DisposableStore): Event<T> {
506516
let buffer: T[] | null = _buffer.slice();
507517

518+
// Dev-only leak detection: track when buffer was created and warn
519+
// if events accumulate without ever being consumed.
520+
let bufferLeakWarningData: { stack: Stacktrace; timerId: ReturnType<typeof setTimeout>; warned: boolean } | undefined;
521+
if (_isBufferLeakWarningEnabled()) {
522+
bufferLeakWarningData = {
523+
stack: Stacktrace.create(),
524+
timerId: setTimeout(() => {
525+
if (buffer && buffer.length > 0 && bufferLeakWarningData && !bufferLeakWarningData.warned) {
526+
bufferLeakWarningData.warned = true;
527+
console.warn(`[Event.buffer][${debugName}] potential LEAK detected: ${buffer.length} events buffered for ${_bufferLeakWarnTimeThreshold / 1000}s without being consumed. Buffered here:`);
528+
bufferLeakWarningData.stack.print();
529+
}
530+
}, _bufferLeakWarnTimeThreshold),
531+
warned: false
532+
};
533+
if (disposable) {
534+
disposable.add(toDisposable(() => clearTimeout(bufferLeakWarningData!.timerId)));
535+
}
536+
}
537+
538+
const clearLeakWarningTimer = () => {
539+
if (bufferLeakWarningData) {
540+
clearTimeout(bufferLeakWarningData.timerId);
541+
}
542+
};
543+
508544
let listener: IDisposable | null = event(e => {
509545
if (buffer) {
510546
buffer.push(e);
547+
if (_isBufferLeakWarningEnabled() && bufferLeakWarningData && !bufferLeakWarningData.warned && buffer.length >= _bufferLeakWarnCountThreshold) {
548+
bufferLeakWarningData.warned = true;
549+
console.warn(`[Event.buffer][${debugName}] potential LEAK detected: ${buffer.length} events buffered without being consumed. Buffered here:`);
550+
bufferLeakWarningData.stack.print();
551+
}
511552
} else {
512553
emitter.fire(e);
513554
}
@@ -520,6 +561,7 @@ export namespace Event {
520561
const flush = () => {
521562
buffer?.forEach(e => emitter.fire(e));
522563
buffer = null;
564+
clearLeakWarningTimer();
523565
};
524566

525567
const emitter = new Emitter<T>({
@@ -547,6 +589,7 @@ export namespace Event {
547589
listener.dispose();
548590
}
549591
listener = null;
592+
clearLeakWarningTimer();
550593
}
551594
});
552595

‎src/vs/base/parts/ipc/common/ipc.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,7 @@ export namespace ProxyChannel {
11181118
const mapEventNameToEvent = new Map<string, Event<unknown>>();
11191119
for (const key in handler) {
11201120
if (propertyIsEvent(key)) {
1121-
mapEventNameToEvent.set(key, Event.buffer(handler[key] as Event<unknown>, true, undefined, disposables));
1121+
mapEventNameToEvent.set(key, Event.buffer(handler[key] as Event<unknown>, key, true, undefined, disposables));
11221122
}
11231123
}
11241124

@@ -1137,7 +1137,7 @@ export namespace ProxyChannel {
11371137
}
11381138

11391139
if (propertyIsEvent(event)) {
1140-
mapEventNameToEvent.set(event, Event.buffer(handler[event] as Event<unknown>, true, undefined, disposables));
1140+
mapEventNameToEvent.set(event, Event.buffer(handler[event] as Event<unknown>, event, true, undefined, disposables));
11411141

11421142
return mapEventNameToEvent.get(event) as Event<T>;
11431143
}

‎src/vs/base/test/common/event.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,7 @@ suite('Event utils', () => {
10061006
const result: number[] = [];
10071007
const emitter = ds.add(new Emitter<number>());
10081008
const event = emitter.event;
1009-
const bufferedEvent = Event.buffer(event);
1009+
const bufferedEvent = Event.buffer(event, 'test');
10101010

10111011
emitter.fire(1);
10121012
emitter.fire(2);
@@ -1028,7 +1028,7 @@ suite('Event utils', () => {
10281028
const result: number[] = [];
10291029
const emitter = ds.add(new Emitter<number>());
10301030
const event = emitter.event;
1031-
const bufferedEvent = Event.buffer(event, true);
1031+
const bufferedEvent = Event.buffer(event, 'test', true);
10321032

10331033
emitter.fire(1);
10341034
emitter.fire(2);
@@ -1050,7 +1050,7 @@ suite('Event utils', () => {
10501050
const result: number[] = [];
10511051
const emitter = ds.add(new Emitter<number>());
10521052
const event = emitter.event;
1053-
const bufferedEvent = Event.buffer(event, false, [-2, -1, 0]);
1053+
const bufferedEvent = Event.buffer(event, 'test', false, [-2, -1, 0]);
10541054

10551055
emitter.fire(1);
10561056
emitter.fire(2);

‎src/vs/platform/extensionManagement/common/extensionManagementIpc.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ export class ExtensionManagementChannel<TContext = RemoteAgentConnectionContext
5454
readonly onDidUpdateExtensionMetadata: Event<DidUpdateExtensionMetadata>;
5555

5656
constructor(private service: IExtensionManagementService, private getUriTransformer: (requestContext: TContext) => IURITransformer | null) {
57-
this.onInstallExtension = Event.buffer(service.onInstallExtension, true);
58-
this.onDidInstallExtensions = Event.buffer(service.onDidInstallExtensions, true);
59-
this.onUninstallExtension = Event.buffer(service.onUninstallExtension, true);
60-
this.onDidUninstallExtension = Event.buffer(service.onDidUninstallExtension, true);
61-
this.onDidUpdateExtensionMetadata = Event.buffer(service.onDidUpdateExtensionMetadata, true);
57+
this.onInstallExtension = Event.buffer(service.onInstallExtension, 'onInstallExtension', true);
58+
this.onDidInstallExtensions = Event.buffer(service.onDidInstallExtensions, 'onDidInstallExtensions', true);
59+
this.onUninstallExtension = Event.buffer(service.onUninstallExtension, 'onUninstallExtension', true);
60+
this.onDidUninstallExtension = Event.buffer(service.onDidUninstallExtension, 'onDidUninstallExtension', true);
61+
this.onDidUpdateExtensionMetadata = Event.buffer(service.onDidUpdateExtensionMetadata, 'onDidUpdateExtensionMetadata', true);
6262
}
6363

6464
// eslint-disable-next-line @typescript-eslint/no-explicit-any

‎src/vs/platform/mcp/common/mcpManagementIpc.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ export class McpManagementChannel<TContext = RemoteAgentConnectionContext | stri
4646
readonly onDidUninstallMcpServer: Event<DidUninstallMcpServerEvent>;
4747

4848
constructor(private service: IMcpManagementService, private getUriTransformer: (requestContext: TContext) => IURITransformer | null) {
49-
this.onInstallMcpServer = Event.buffer(service.onInstallMcpServer, true);
50-
this.onDidInstallMcpServers = Event.buffer(service.onDidInstallMcpServers, true);
51-
this.onDidUpdateMcpServers = Event.buffer(service.onDidUpdateMcpServers, true);
52-
this.onUninstallMcpServer = Event.buffer(service.onUninstallMcpServer, true);
53-
this.onDidUninstallMcpServer = Event.buffer(service.onDidUninstallMcpServer, true);
49+
this.onInstallMcpServer = Event.buffer(service.onInstallMcpServer, 'onInstallMcpServer', true);
50+
this.onDidInstallMcpServers = Event.buffer(service.onDidInstallMcpServers, 'onDidInstallMcpServers', true);
51+
this.onDidUpdateMcpServers = Event.buffer(service.onDidUpdateMcpServers, 'onDidUpdateMcpServers', true);
52+
this.onUninstallMcpServer = Event.buffer(service.onUninstallMcpServer, 'onUninstallMcpServer', true);
53+
this.onDidUninstallMcpServer = Event.buffer(service.onDidUninstallMcpServer, 'onDidUninstallMcpServer', true);
5454
}
5555

5656
listen<T>(context: TContext, event: string): Event<T> {

‎src/vs/workbench/api/common/extHostExtensionService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ export abstract class AbstractExtHostExtensionService extends Disposable impleme
557557
return undefined;
558558
}
559559

560-
const onDidReceiveMessage = Event.buffer(Event.fromDOMEventEmitter(messagePort, 'message', e => e.data));
560+
const onDidReceiveMessage = Event.buffer(Event.fromDOMEventEmitter(messagePort, 'message', e => e.data), 'onDidReceiveMessage');
561561
messagePort.start();
562562
messagePassingProtocol = {
563563
onDidReceiveMessage,

‎src/vs/workbench/services/terminal/common/embedderTerminalService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class EmbedderTerminalService implements IEmbedderTerminalService {
5656
declare _serviceBrand: undefined;
5757

5858
private readonly _onDidCreateTerminal = new Emitter<IShellLaunchConfig>();
59-
readonly onDidCreateTerminal = Event.buffer(this._onDidCreateTerminal.event);
59+
readonly onDidCreateTerminal = Event.buffer(this._onDidCreateTerminal.event, 'onDidCreateTerminal');
6060

6161
createTerminal(options: IEmbedderTerminalOptions): void {
6262
const slc: EmbedderTerminal = {

0 commit comments

Comments
 (0)