Fix timeline view leaks event listeners (microsoft#241607)

* Fixes microsoft#241536

* Feedback

* cleanup

* fix hygieene

---------

Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>

Author: jeanp413

src/vs/workbench/contrib/timeline/browser/timelinePane.ts

@@ -235,6 +235,10 @@ export const TimelineFollowActiveEditorContext = new RawContextKey<boolean>('tim
 export const TimelineExcludeSources = new RawContextKey<string>('timelineExcludeSources', '[]', true);
 export const TimelineViewFocusedContext = new RawContextKey<boolean>('timelineFocused', true);
 
+interface IPendingRequest extends IDisposable {
+	readonly request: TimelineRequest;
+}
+
 export class TimelinePane extends ViewPane {
 	static readonly TITLE: ILocalizedString = localize2('timeline', "Timeline");
 
@@ -250,7 +254,7 @@ export class TimelinePane extends ViewPane {
 	private timelineExcludeSourcesContext: IContextKey<string>;
 
 	private excludedSources: Set<string>;
-	private pendingRequests = new Map<string, TimelineRequest>();
+	private pendingRequests = new Map<string, IPendingRequest>();
 	private timelinesBySource = new Map<string, TimelineAggregate>();
 
 	private uri: URI | undefined;
@@ -495,8 +499,9 @@ export class TimelinePane extends ViewPane {
 		this.timelinesBySource.clear();
 
 		if (cancelPending) {
-			for (const { tokenSource } of this.pendingRequests.values()) {
-				tokenSource.dispose(true);
+			for (const pendingRequest of this.pendingRequests.values()) {
+				pendingRequest.request.tokenSource.cancel();
+				pendingRequest.dispose();
 			}
 
 			this.pendingRequests.clear();
@@ -588,34 +593,38 @@ export class TimelinePane extends ViewPane {
 			options = { cursor: reset ? undefined : timeline?.cursor, limit: this.pageSize };
 		}
 
-		let request = this.pendingRequests.get(source);
-		if (request !== undefined) {
-			options.cursor = request.options.cursor;
+		const pendingRequest = this.pendingRequests.get(source);
+		if (pendingRequest !== undefined) {
+			options.cursor = pendingRequest.request.options.cursor;
 
 			// TODO@eamodio deal with concurrent requests better
 			if (typeof options.limit === 'number') {
-				if (typeof request.options.limit === 'number') {
-					options.limit += request.options.limit;
+				if (typeof pendingRequest.request.options.limit === 'number') {
+					options.limit += pendingRequest.request.options.limit;
 				} else {
-					options.limit = request.options.limit;
+					options.limit = pendingRequest.request.options.limit;
 				}
 			}
 		}
-		request?.tokenSource.dispose(true);
+		pendingRequest?.request?.tokenSource.cancel();
+		pendingRequest?.dispose();
+
 		options.cacheResults = true;
 		options.resetCache = reset;
-		request = this.timelineService.getTimeline(
-			source, uri, options, new CancellationTokenSource()
-		);
+		const tokenSource = new CancellationTokenSource();
+		const newRequest = this.timelineService.getTimeline(source, uri, options, tokenSource);
 
-		if (request === undefined) {
+		if (newRequest === undefined) {
+			tokenSource.dispose();
 			return false;
 		}
 
-		this.pendingRequests.set(source, request);
-		request.tokenSource.token.onCancellationRequested(() => this.pendingRequests.delete(source));
+		const disposables = new DisposableStore();
+		this.pendingRequests.set(source, { request: newRequest, dispose: () => disposables.dispose() });
+		disposables.add(tokenSource);
+		disposables.add(tokenSource.token.onCancellationRequested(() => this.pendingRequests.delete(source)));
 
-		this.handleRequest(request);
+		this.handleRequest(newRequest);
 
 		return true;
 	}
@@ -641,6 +650,7 @@ export class TimelinePane extends ViewPane {
 			response = await this.progressService.withProgress({ location: this.id }, () => request.result);
 		}
 		finally {
+			this.pendingRequests.get(request.source)?.dispose();
 			this.pendingRequests.delete(request.source);
 		}
 
@@ -920,11 +930,11 @@ export class TimelinePane extends ViewPane {
 		container.appendChild(this.$tree);
 
 		this.treeRenderer = this.instantiationService.createInstance(TimelineTreeRenderer, this.commands);
-		this.treeRenderer.onDidScrollToEnd(item => {
+		this._register(this.treeRenderer.onDidScrollToEnd(item => {
 			if (this.pageOnScroll) {
 				this.loadMore(item);
 			}
-		});
+		}));
 
 		this.tree = this.instantiationService.createInstance(WorkbenchObjectTree<TreeElement, FuzzyScore>, 'TimelinePane',
 			this.$tree, new TimelineListVirtualDelegate(), [this.treeRenderer], {

src/vs/workbench/contrib/timeline/common/timelineService.ts

@@ -4,8 +4,8 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { CancellationTokenSource } from '../../../../base/common/cancellation.js';
-import { Event, Emitter } from '../../../../base/common/event.js';
-import { IDisposable } from '../../../../base/common/lifecycle.js';
+import { Emitter } from '../../../../base/common/event.js';
+import { Disposable, IDisposable } from '../../../../base/common/lifecycle.js';
 import { URI } from '../../../../base/common/uri.js';
 import { ILogService } from '../../../../platform/log/common/log.js';
 import { ITimelineService, TimelineChangeEvent, TimelineOptions, TimelineProvidersChangeEvent, TimelineProvider, TimelinePaneId } from './timeline.js';
@@ -15,16 +15,16 @@ import { IContextKey, IContextKeyService, RawContextKey } from '../../../../plat
 
 export const TimelineHasProviderContext = new RawContextKey<boolean>('timelineHasProvider', false);
 
-export class TimelineService implements ITimelineService {
+export class TimelineService extends Disposable implements ITimelineService {
 	declare readonly _serviceBrand: undefined;
 
-	private readonly _onDidChangeProviders = new Emitter<TimelineProvidersChangeEvent>();
-	readonly onDidChangeProviders: Event<TimelineProvidersChangeEvent> = this._onDidChangeProviders.event;
+	private readonly _onDidChangeProviders = this._register(new Emitter<TimelineProvidersChangeEvent>());
+	readonly onDidChangeProviders = this._onDidChangeProviders.event;
 
-	private readonly _onDidChangeTimeline = new Emitter<TimelineChangeEvent>();
-	readonly onDidChangeTimeline: Event<TimelineChangeEvent> = this._onDidChangeTimeline.event;
-	private readonly _onDidChangeUri = new Emitter<URI>();
-	readonly onDidChangeUri: Event<URI> = this._onDidChangeUri.event;
+	private readonly _onDidChangeTimeline = this._register(new Emitter<TimelineChangeEvent>());
+	readonly onDidChangeTimeline = this._onDidChangeTimeline.event;
+	private readonly _onDidChangeUri = this._register(new Emitter<URI>());
+	readonly onDidChangeUri = this._onDidChangeUri.event;
 
 	private readonly hasProviderContext: IContextKey<boolean>;
 	private readonly providers = new Map<string, TimelineProvider>();
@@ -36,6 +36,8 @@ export class TimelineService implements ITimelineService {
 		@IConfigurationService protected configurationService: IConfigurationService,
 		@IContextKeyService protected contextKeyService: IContextKeyService,
 	) {
+		super();
+
 		this.hasProviderContext = TimelineHasProviderContext.bindTo(this.contextKeyService);
 		this.updateHasProviderContext();
 	}
@@ -72,10 +74,10 @@ export class TimelineService implements ITimelineService {
 
 					return result;
 				}),
-			options: options,
+			options,
 			source: provider.id,
-			tokenSource: tokenSource,
-			uri: uri
+			tokenSource,
+			uri
 		};
 	}
 
@@ -120,6 +122,7 @@ export class TimelineService implements ITimelineService {
 		}
 
 		this.providers.delete(id);
+		this.providerSubscriptions.get(id)?.dispose();
 		this.providerSubscriptions.delete(id);
 
 		this.updateHasProviderContext();