proposal-explicit-resource-management: `AsyncDisposableStack.use()` cannot dispose `Disposable` synchronously

The AsyncDisposableStack.use() is equivalent to await using. But there’s no API equivalent to using on AsyncDisposableStack. This can lead to subtle behavior difference if there’s async race:

import { describe, expect, test, jest } from "@jest/globals";
require("disposablestack/auto");

function listen(target: EventTarget, event: string, callback: (e: Event) => void): Disposable {
    target.addEventListener(event, callback);
    return { [Symbol.dispose]() { target.removeEventListener(event, callback); } };
}

function race(target: EventTarget, event: Event, resolve: () => void): Disposable {
    return { [Symbol.dispose]() {
        Promise.resolve().then(() => {
            target.dispatchEvent(event);
            resolve();
        });
    } };
}

class MyEvent extends Event {
    constructor(name: string, public n: number) {
        super(name);
    }
}

describe("Async race in dispose", () => {
    test("using", async () => {
        const log = jest.fn();
        await expect(new Promise<void>((resolve) => {
            const target = new EventTarget();
            using listener = listen(target, "event", ({ n }: any) => { log(n) });
            using _ = race(target, new MyEvent("event", 2), resolve);
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1]]);
    });

    test("await using", async () => {
        const log = jest.fn();
        await expect(new Promise<void>(async (resolve) => {
            const target = new EventTarget();
            await using listener = listen(target, "event", ({ n }: any) => { log(n) });
            await using _ = race(target, new MyEvent("event", 2), resolve);
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1], [2]]);
    });

    test("DisposableStack", async () => {
        const log = jest.fn();
        await expect(new Promise<void>((resolve) => {
            using stack = new DisposableStack();
            const target = new EventTarget();
            stack.use(listen(target, "event", ({ n }: any) => { log(n) }));
            stack.use(race(target, new MyEvent("event", 2), resolve));
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1]]);
    });

    test("AsyncDisposableStack", async () => {
        const log = jest.fn();
        await expect(new Promise<void>(async (resolve) => {
            await using stack = new AsyncDisposableStack();
            const target = new EventTarget();
            stack.use(listen(target, "event", ({ n }: any) => { log(n) }));
            stack.use(race(target, new MyEvent("event", 2), resolve));
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1], [2]]);
    });
});

About this issue

  • Original URL
  • State: open
  • Created a year ago
  • Comments: 17 (6 by maintainers)

Most upvoted comments

I was looking into this not too long ago. The approach I had considered was to add a slot to DisposeCapability indicating whether there were any await using declarations in the block (regardless as to the actual resource value), merging CreateDisposableResource and GetDisposeMethod such that only resources with @@asyncDispose are added with an async-dispose hint, and modifying DisposeResources to track whether there were any async-dispose resources that are disposed and, if there are none and the DisposeCapability contained any await using declarations then an implicit await undefined would occur. An AsyncDisposableStack wouldn’t necessarily need to set the new slot on DisposeCapability since it’s @@asyncDispose always returns a Promise anyways.

IMO, that would result in far clearer spec text than the current approach with respect to await using x = null, since that just adds a resource with no value or method just to trigger an await.

function race(target: EventTarget, event: Event, resolve: () => void): Disposable {
    return { [Symbol.dispose]() {
        Promise.resolve().then(() => {
            target.dispatchEvent(event);
            resolve();
        });
    } };
}

Well there is your problem. If your Disposable is doing async work in its dispose, it really should be an AsyncDisposable. I don’t see a way around this.

race()'s dispose is not actually doing async work. It returns synchronously. The dispatch event can be a side-effect of disposing race(). In fact the side-effect can happen anywhere before we enter the disposal of listen(), even inside the main function body. I’m just illustrating one case for demonstration purpose.

This can be solved if we provide a synchronous version of stack.use(), so people can choose to use the correct one, just like in an async function you can choose to use using or await using for a Disposable type.

import { describe, expect, test, jest } from "@jest/globals";
require("disposablestack/auto");
const SLOT = require('internal-slot');

class ExtendedAsyncDisposableStack extends AsyncDisposableStack {
    constructor() {
        super();
        SLOT.set(this, '[[DisposableState]]', 'pending');
    }
    useSync(disposable: Disposable): void {
        DisposableStack.prototype.use.call(this, disposable);
    }
    disposeAsync(): Promise<void> {
        SLOT.set(this, '[[DisposableState]]', 'disposed');
        return super.disposeAsync();
    }
    move(): AsyncDisposableStack {
        SLOT.set(this, '[[DisposableState]]', 'disposed');
        return super.move();
    }
}

function listen(target: EventTarget, event: string, callback: (e: Event) => void): Disposable {
    target.addEventListener(event, callback);
    return { [Symbol.dispose]() { target.removeEventListener(event, callback); } };
}

function race(target: EventTarget, event: Event, resolve: () => void): Disposable {
    return { [Symbol.dispose]() {
        Promise.resolve().then(() => {
            target.dispatchEvent(event);
            resolve();
        });
    } };
}

class MyEvent extends Event {
    constructor(name: string, public n: number) {
        super(name);
    }
}

describe("Async race in dispose", () => {
    test("ExtendedAsyncDisposableStack with useSync()", async () => {
        const log = jest.fn();
        await expect(new Promise<void>(async (resolve) => {
            await using stack = new ExtendedAsyncDisposableStack();
            const target = new EventTarget();
            stack.useSync(listen(target, "event", ({ n }: any) => { log(n) }));
            stack.useSync(race(target, new MyEvent("event", 2), resolve));
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1]]);
    });

    test("ExtendedAsyncDisposableStack with async use()", async () => {
        const log = jest.fn();
        await expect(new Promise<void>(async (resolve) => {
            await using stack = new ExtendedAsyncDisposableStack();
            const target = new EventTarget();
            stack.use(listen(target, "event", ({ n }: any) => { log(n) }));
            stack.use(race(target, new MyEvent("event", 2), resolve));
            target.dispatchEvent(new MyEvent("event", 1));
        })).resolves.not.toThrow();
        expect(log.mock.calls).toStrictEqual([[1], [2]]);
    });
});