diff --git a/src/browser/decorations/BufferDecorationRenderer.ts b/src/browser/decorations/BufferDecorationRenderer.ts index 72ed6428..d33115f7 100644 --- a/src/browser/decorations/BufferDecorationRenderer.ts +++ b/src/browser/decorations/BufferDecorationRenderer.ts @@ -98,7 +98,6 @@ export class BufferDecorationRenderer extends Disposable { } else { let element = this._decorationElements.get(decoration); if (!element) { - decoration.onDispose(() => this._removeDecoration(decoration)); element = this._createElement(decoration); decoration.element = element; this._decorationElements.set(decoration, element); @@ -125,5 +124,6 @@ export class BufferDecorationRenderer extends Disposable { private _removeDecoration(decoration: IInternalDecoration): void { this._decorationElements.get(decoration)?.remove(); this._decorationElements.delete(decoration); + decoration.dispose(); } } diff --git a/src/common/Lifecycle.ts b/src/common/Lifecycle.ts index 7ccc8aa7..b3a7cc21 100644 --- a/src/common/Lifecycle.ts +++ b/src/common/Lifecycle.ts @@ -17,18 +17,15 @@ export abstract class Disposable implements IDisposable { } /** - * Disposes the object, triggering the `dispose` method on all registered IDisposables. This is a - * readonly property instead of a method to prevent subclasses overriding it which is an easy - * mistake that can introduce memory leaks. If a class extends Disposable, all dispose calls - * should be done via {@link register}. + * Disposes the object, triggering the `dispose` method on all registered IDisposables. */ - public readonly dispose = (): void => { + public dispose(): void { this._isDisposed = true; for (const d of this._disposables) { d.dispose(); } this._disposables.length = 0; - }; + } /** * Registers a disposable object. diff --git a/src/common/services/DecorationService.ts b/src/common/services/DecorationService.ts index c27e7b2f..881b3d07 100644 --- a/src/common/services/DecorationService.ts +++ b/src/common/services/DecorationService.ts @@ -42,6 +42,7 @@ export class DecorationService extends Disposable implements IDecorationService this.reset(); })); } + public registerDecoration(options: IDecorationOptions): IDecoration | undefined { if (options.marker.isDisposed) { return undefined; @@ -91,19 +92,25 @@ export class DecorationService extends Disposable implements IDecorationService } }); } + + public dispose(): void { + for (const d of this._decorations.values()) { + this._onDecorationRemoved.fire(d); + } + this.reset(); + } } class Decoration extends Disposable implements IInternalDecoration { public readonly marker: IMarker; public element: HTMLElement | undefined; + public isDisposed: boolean = false; public readonly onRenderEmitter = this.register(new EventEmitter()); public readonly onRender = this.onRenderEmitter.event; private readonly _onDispose = this.register(new EventEmitter()); public readonly onDispose = this._onDispose.event; - public get isDisposed(): boolean { return this._isDisposed; } - private _cachedBg: IColor | undefined | null = null; public get backgroundColorRGB(): IColor | undefined { if (this._cachedBg === null) { @@ -136,12 +143,10 @@ class Decoration extends Disposable implements IInternalDecoration { if (this.options.overviewRulerOptions && !this.options.overviewRulerOptions.position) { this.options.overviewRulerOptions.position = 'full'; } + } - this.register(toDisposable(() => { - if (this._isDisposed) { - return; - } - this._onDispose.fire(); - })); + public override dispose(): void { + this._onDispose.fire(); + super.dispose(); } }