diff --git a/packages/utils-disposables/src/DisposableList.test.ts b/packages/utils-disposables/src/DisposableList.test.ts new file mode 100644 index 0000000000..cfd316d932 --- /dev/null +++ b/packages/utils-disposables/src/DisposableList.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, test } from '@jest/globals'; + +import { type DisposableLike, disposeOf, isDisposableHybrid, isDisposed } from './disposable.js'; +import { createDisposableList, DisposableList, InheritableDisposable } from './DisposableList.js'; + +describe('disposable', () => { + test('InheritableDisposable', () => { + let count = 0; + class MyDisposable extends InheritableDisposable { + constructor(disposables: DisposableLike[]) { + super(disposables); + } + } + + function use() { + using _d = new MyDisposable([() => (count += 10)]); + } + + expect(count).toBe(0); + + use(); + + expect(count).toBe(10); + }); + + test('DisposableList', () => { + let count = 0; + + function use() { + using list = new DisposableList([() => (count += 10)]); + list.push(() => (count += 100)); + } + + expect(count).toBe(0); + + use(); + + expect(count).toBe(110); + }); + + test('createDisposableList', () => { + const list = createDisposableList(); + let count = 0; + list.push(() => (count += 1)); + disposeOf(list); + expect(count).toBe(1); + expect(isDisposed(list)).toBe(true); + expect(list.isDisposed()).toBe(true); + }); + + test('double dispose', () => { + let count = 0; + const list = createDisposableList([() => (count += 10)]); + function use() { + using aliasList = list; + aliasList.push(() => (count += 100)); + } + + expect(list.length).toBe(1); + use(); + + expect(count).toBe(110); + expect(list.length).toBe(0); + + expect(() => list.push(() => (count += 1000))).toThrowError('Already disposed, cannot add items.'); + + expect(list.length).toBe(0); + list.dispose(); + expect(list.length).toBe(0); + + expect(count).toBe(110); + }); + + test('', () => { + const list = createDisposableList(); + expect(isDisposableHybrid(list)).toBe(true); + expect(list.isDisposed()); + }); +}); diff --git a/packages/utils-disposables/src/DisposableList.ts b/packages/utils-disposables/src/DisposableList.ts new file mode 100644 index 0000000000..ebce341b83 --- /dev/null +++ b/packages/utils-disposables/src/DisposableList.ts @@ -0,0 +1,60 @@ +import type { DisposableHybrid, DisposableLike } from './disposable.js'; +import { createDisposeMethodFromList, symbolIsDisposed } from './disposable.js'; + +/** This is a class that can be inherited to provide Disposable support. */ + +export const noop = () => undefined; + +export class InheritableDisposable implements DisposableHybrid { + public dispose: () => void; + public [Symbol.dispose]: () => void = noop; + public [symbolIsDisposed]: boolean = false; + + /** the inherited class can safely add disposables to _disposables */ + protected readonly _disposables: DisposableLike[]; + constructor(disposables?: DisposableLike[], name = 'InheritableDisposable') { + this._disposables = disposables ?? []; + const _dispose = createDisposeMethodFromList(this._disposables, name); + const dispose = () => { + if (this.isDisposed()) return; + this[symbolIsDisposed] = true; + _dispose(); + // Prevent new disposables from being added. + Object.freeze(this._disposables); + }; + this.dispose = dispose; + this[Symbol.dispose] = dispose; + } + + protected isDisposed(): boolean { + return this[symbolIsDisposed]; + } +} + +export class DisposableList extends InheritableDisposable { + constructor( + public readonly disposables: DisposableLike[] = [], + readonly name = 'DisposableList', + ) { + super(disposables); + } + + public push(disposable: DisposableLike) { + if (this.isDisposed()) { + throw new Error('Already disposed, cannot add items.'); + } + this.disposables.push(disposable); + } + + get length() { + return this.disposables.length; + } + + public isDisposed(): boolean { + return super.isDisposed(); + } +} + +export function createDisposableList(disposables?: DisposableLike[], name?: string): DisposableList { + return new DisposableList(disposables, name); +} diff --git a/packages/utils-disposables/src/disposable.test.ts b/packages/utils-disposables/src/disposable.test.ts index a7aa4e66b1..a814081276 100644 --- a/packages/utils-disposables/src/disposable.test.ts +++ b/packages/utils-disposables/src/disposable.test.ts @@ -1,7 +1,20 @@ import { describe, expect, jest, test } from '@jest/globals'; -import type { DisposableLike } from './disposable.js'; -import { createDisposable, createDisposableFromList, InheritableDisposable, injectDisposable } from './disposable.js'; +import type { DisposableLike, DisposeFn } from './disposable.js'; +import { + createDisposable, + createDisposableFromList, + createDisposeMethodFromList, + disposeOf, + getDisposableName, + injectDisposable, + isDisposableHybrid, + isDisposed, + makeDisposable, + setDebugMode, + setDisposableName, +} from './disposable.js'; +import { InheritableDisposable } from './DisposableList.js'; describe('disposable', () => { test('createDisposable', () => { @@ -15,6 +28,20 @@ describe('disposable', () => { expect(dispose).toHaveBeenCalledTimes(1); }); + test('createDisposable named', () => { + const dispose = jest.fn(); + const myDisposable = createDisposable(dispose, undefined, 'MyDisposable'); + + function use() { + using _obj = myDisposable; + } + expect(isDisposed(myDisposable)).toBe(false); + use(); + expect(isDisposed(myDisposable)).toBe(true); + expect(getDisposableName(myDisposable)).toBe('MyDisposable'); + expect(dispose).toHaveBeenCalledTimes(1); + }); + test('createDisposable thisArg', () => { const myObj = { callMe: jest.fn(), @@ -108,4 +135,113 @@ describe('disposable', () => { expect(count).toBe(10); }); + + test.each` + value | expected + ${undefined} | ${false} + ${null} | ${false} + ${1} | ${false} + ${'hello'} | ${false} + ${{}} | ${false} + ${makeDisposable(() => undefined)} | ${true} + `('isDisposableHybrid', ({ value, expected }) => { + expect(isDisposableHybrid(value)).toBe(expected); + }); + + test('makeDisposable', () => { + let count = 0; + const a1 = () => (count += 1); + const d1 = makeDisposable(a1); + expect(isDisposableHybrid(d1)).toBe(true); + expect(isDisposed(d1)).toBe(false); + expect(makeDisposable(d1)).toBe(d1); + expect(count).toBe(0); + expect(getDisposableName(d1)).toBe('makeDisposable'); + + const a2 = { dispose: () => (count += 10) }; + const d2 = makeDisposable(a2); + expect(isDisposableHybrid(d2)).toBe(true); + + const a3 = { [Symbol.dispose]: () => (count += 100) }; + const d3 = makeDisposable(a3); + expect(isDisposableHybrid(d3)).toBe(true); + + const a4 = { [Symbol.dispose]: a1, dispose: a1 }; + const d4 = makeDisposable(a4, 'a4'); + expect(isDisposableHybrid(d4)).toBe(true); + expect(getDisposableName(d4)).toBe('a4'); + + d4.dispose(); + expect(count).toBe(1); + }); + + test('get/set name', () => { + const d = createDisposable(() => undefined, undefined, 'name1'); + expect(getDisposableName(d)).toBe('name1'); + setDisposableName(d, 'name2'); + expect(getDisposableName(d)).toBe('name2'); + }); +}); + +describe('disposable debug', () => { + beforeEach(() => { + setDebugMode(true); + jest.spyOn(console, 'log').mockImplementation(() => undefined); + jest.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + setDebugMode(false); + jest.resetAllMocks(); + }); + + test('createDisposableFromList', () => { + let count = 0; + const disposables = [ + createDisposable(() => (count += 1)), + createDisposable(() => (count += 10)), + createDisposable(() => (count += 100)), + ]; + function use() { + using _obj = createDisposableFromList(disposables); + } + use(); + expect(count).toBe(111); + }); + + test('dispose with errors', () => { + const e1 = Error('one'); + const e2 = Error('two'); + + const d = createDisposableFromList([ + () => { + throw e1; + }, + () => { + throw e2; + }, + ]); + + expect(() => disposeOf(d)).toThrow(e2); + }); + + test('createDisposableFromList double dispose', () => { + const list: DisposeFn[] = []; + const d = createDisposableFromList(list); + d.dispose(); + list.push(() => undefined); + d.dispose(); + // It was not disposed. + expect(list.length).toBe(1); + }); + + test('createDisposableFromList double dispose', () => { + const list: DisposeFn[] = []; + const d = createDisposeMethodFromList(list); + d(); + list.push(() => undefined); + d(); + // It was disposed. + expect(list.length).toBe(0); + }); }); diff --git a/packages/utils-disposables/src/disposable.ts b/packages/utils-disposables/src/disposable.ts index 090ff16414..0c8e8c9597 100644 --- a/packages/utils-disposables/src/disposable.ts +++ b/packages/utils-disposables/src/disposable.ts @@ -9,6 +9,27 @@ interface Disposable { [Symbol.dispose](): void; } +export const symbolDisposableName = Symbol('Disposable Name'); +export type SymbolDisposableName = typeof symbolDisposableName; + +export const symbolDisposableTs = Symbol('Disposable Timestamp'); +export type SymbolDisposableTs = typeof symbolDisposableTs; + +export const symbolIsDisposed = Symbol('Disposable Is Disposed'); +export type SymbolIsDisposed = typeof symbolIsDisposed; + +let debugMode = false; + +let debugDepth = 0; +let activeDisposables = 0; + +function logDebug(...params: Parameters): void { + if (!debugMode) return; + const [msg, ...rest] = params; + + console.log(' '.repeat(debugDepth) + msg, ...rest); +} + export type DisposeFn = () => void; export interface DisposableHybrid { @@ -17,6 +38,20 @@ export interface DisposableHybrid { */ dispose(): void; [Symbol.dispose](): void; + /** + * The name of the disposable, used for debugging purposes. + * It can be helpful to trace the origins of the disposable. + */ + [symbolDisposableName]?: string; + /** + * The timestamp, see: [Performance.now()](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now) + */ + [symbolDisposableTs]?: number; + + /** + * Indicates if the disposable has been disposed. + */ + [symbolIsDisposed]?: boolean; } export interface DisposableClassic { @@ -39,65 +74,127 @@ export type DisposableLike = DisposableHybrid | DisposableClassic | DisposablePr * Create a Disposable object. * @param disposeFn - function to call when this option is disposed. * @param thisArg - optional this value + * @param name - optional debug name * @returns A Disposable */ -export function createDisposable(disposeFn: DisposeFn, thisArg?: T): DisposableHybrid { +export function createDisposable(disposeFn: DisposeFn, thisArg?: T, name?: string): DisposableHybrid { // We want to prevent double disposal calls. // This can happen if there are multiple systems calling dispose. let isDisposed = false; + let errors = false; - function dispose() { - if (isDisposed) return; - isDisposed = true; - thisArg ? disposeFn.call(thisArg) : disposeFn(); - } - - return { + const disposable: DisposableHybrid = { dispose, [Symbol.dispose]: dispose, + [symbolDisposableTs]: performance.now(), + [symbolDisposableName]: name || undefined, + [symbolIsDisposed]: false, }; + + ++activeDisposables; + debugMode && logDebug('Created: %s, active: %i', debugId(disposable), activeDisposables); + + return disposable; + + function dispose() { + try { + debugMode && logDebug('Dispose Start -> %s Active %i', debugId(disposable), activeDisposables); + ++debugDepth; + // isDisposed is the source of truth, not `disposable[symbolIsDisposed]` + if (isDisposed) { + disposable[symbolIsDisposed] = true; + debugMode && console.error('Already disposed %s', debugId(disposable)); + return; + } + --activeDisposables; + disposable[symbolIsDisposed] = isDisposed = true; + thisArg ? disposeFn.call(thisArg) : disposeFn(); + } catch (err) { + errors = true; + throw err; + } finally { + --debugDepth; + debugMode && + logDebug('Dispose End -> %s Active %i%s', debugId(disposable), activeDisposables, errors ? ' ** with errors ** ' : ''); + } + } +} + +function debugId(disposable: DisposableHybrid): string { + const name = getDisposableName(disposable) || ''; + const ts = getDisposableTs(disposable)?.toFixed(4); + return name + ' ' + ts; } /** * Make and object Disposable by adding disposable properties. * @param obj - Object to modify * @param dispose - the dispose function. + * @param name - optional debug name * @returns the same object. */ -export function injectDisposable(obj: T, dispose: () => void): T & DisposableHybrid { - return Object.assign(obj, createDisposable(dispose, obj)); +export function injectDisposable(obj: T, dispose: () => void, name?: string): T & DisposableHybrid { + return Object.assign(obj, createDisposable(dispose, obj, name)); } /** * Create a Disposable that will dispose the list of disposables. * @param disposables - list of Disposables + * @param name - optional debug name * @returns A Disposable */ -export function createDisposableFromList(disposables: DisposableLike[]): DisposableHybrid { - return createDisposable(createDisposeMethodFromList(disposables)); +export function createDisposableFromList(disposables: DisposableLike[], name = 'createDisposableFromList'): DisposableHybrid { + return createDisposable(createDisposeMethodFromList(disposables), undefined, name); } /** * Create a disposeFn based upon a list of disposables. * @param disposables - list of Disposables + * @param name - optional debug name * @returns A dispose function */ -export function createDisposeMethodFromList(disposables: DisposableLike[]): () => void { +export function createDisposeMethodFromList(disposables: DisposableLike[], name = ''): () => void { + let disposed = false; + const tsId = performance.now().toFixed(4); + + debugMode && (name = 'createDisposeMethodFromList ' + (name || '')); + + debugMode && logDebug('Create: %s %s', name, tsId); + + let errors = 0; + function dispose() { - let error: unknown | undefined = undefined; + try { + debugMode && logDebug('Dispose Start -> %s %s', name, tsId); + ++debugDepth; + let error: unknown | undefined = undefined; + if (disposed) { + debugMode && console.error('Already disposed %s with %o open.', name, disposables.length); + if (!disposables.length) return; + // keep going, try to clean up the list if possible. + } + disposed = true; - let disposable: DisposableLike | undefined; + let disposable: DisposableLike | undefined; - // Note disposables are disposed in reverse order by default. - while ((disposable = disposables.pop())) { - try { - disposeOf(disposable); - } catch (e) { - error ??= e; + // Note disposables are disposed in reverse order by default. + while ((disposable = disposables.pop())) { + try { + disposeOf(disposable); + } catch (e) { + ++errors; + error ??= e; + } } - } - if (error) throw error; + if (error) { + debugMode && console.error(error); + throw error; + } + } finally { + --debugDepth; + debugMode && logDebug('Dispose End -> %s %s%s', name, tsId, errors ? ` *** with ${errors} errors ***` : ''); + } } return dispose; } @@ -121,17 +218,41 @@ export function disposeOf(disposable: DisposableLike | DisposeFn | undefined): v _disposable.dispose.call(disposable); } -/** This is a class that can be inherited to provide Disposable support. */ -export class InheritableDisposable implements DisposableHybrid { - public dispose: () => void; - public [Symbol.dispose]: () => void = () => undefined; +/** + * Make a disposable into a DisposableHybrid + * @param disposable - Disposable Like + * @param name - optional debug name + * @returns DisposableHybrid + */ +export function makeDisposable(disposable: DisposableLike, name = 'makeDisposable'): DisposableHybrid { + if (isDisposableHybrid(disposable)) return disposable; + if (Symbol.dispose in disposable) return createDisposable(disposable[Symbol.dispose], disposable, name); + if ('dispose' in disposable) return createDisposable(disposable['dispose'], disposable, name); + return createDisposable(disposable, undefined, name); +} + +export function setDisposableName(disposable: DisposableHybrid, name: string | undefined): DisposableHybrid { + disposable[symbolDisposableName] = name; + return disposable; +} + +export function getDisposableName(disposable: DisposableHybrid): string | undefined { + return disposable[symbolDisposableName]; +} - /** the inherited class can safely add disposables to _disposables */ - protected readonly _disposables: DisposableLike[]; - constructor(disposables?: DisposableLike[]) { - this._disposables = disposables ?? []; - const dispose = createDisposeMethodFromList(this._disposables); - this.dispose = dispose; - this[Symbol.dispose] = dispose; - } +export function getDisposableTs(disposable: DisposableHybrid): number | undefined { + return disposable[symbolDisposableTs]; +} + +export function isDisposed(disposable: DisposableHybrid): boolean | undefined { + return disposable[symbolIsDisposed]; +} + +export function setDebugMode(enable: boolean) { + debugMode = enable; +} + +export function isDisposableHybrid(disposable: unknown): disposable is DisposableHybrid { + if (!disposable || typeof disposable !== 'object') return false; + return symbolIsDisposed in disposable; } diff --git a/packages/utils-disposables/src/index.ts b/packages/utils-disposables/src/index.ts index 57903b4444..78d2aeda09 100644 --- a/packages/utils-disposables/src/index.ts +++ b/packages/utils-disposables/src/index.ts @@ -4,6 +4,8 @@ export { createDisposableFromList, createDisposeMethodFromList, disposeOf, - InheritableDisposable, injectDisposable, + makeDisposable, + getDisposableTs, } from './disposable.js'; +export { createDisposableList, DisposableList, InheritableDisposable } from './DisposableList.js';