tc39 / proposal-explicit-resource-management

ECMAScript Explicit Resource Management
https://arai-a.github.io/ecma262-compare/?pr=3000
BSD 3-Clause "New" or "Revised" License
726 stars 28 forks source link

Disposable is leaky by default #159

Open rixtox opened 1 year ago

rixtox commented 1 year ago

I'm a bit uneasy with the fact that it's allowed to create a Disposable without registering its dispose method to some kind of disposal scope, either with using on the current scope, or add to some DisposableStack.

In fact, the disposal registration is opt-in, not opt-out. This gives a Disposable object leaky behavior by default. It's like still doing malloc() and free(), but added opt-in support for scope based RAII. This is especially concerning given most of the resource management in JavaScript to date has been implicit by garbage collection. A Disposable object can leak even after it getting garbage collected.

function createHandle(): Disposable & Handle {
    const { write, close } = nativeHandle();
    return { write, [Symbol.dispose](): { close() } };
}

function main() {
    const handle = createHandle();
    handle.write(data);
    // ops you forget to dispose handle
}

main();
// now it's leaking quietly
// any because handle is garbage collected you can't even find it in heap dump

It's worse than leaking - it's leaking quietly, and it can disappear from heap dump because the dispose method can be garbage collected. Diagnosing a leak like this would be a nightmare.

I know, before the proposal people have already been doing explicit resource management in JavaScript. But that's because it's the only way to do it. I would expect a language level improvement on resource management to do better than just providing a syntactic sugar without addressing the unsafe nature of explicit resource management.

I'm not saying I have a solution to the problem. But I'd like to at least start a conversation on possibilities.

My proposal have improved for several iterations thanks to in-depth discussion. To help people to quickly follow up with the most recent proposal of mine, please check this mega post.

Maybe it's better to not exposing the dispose method on any returned value. A dispose method must be registered to some DisposeScope. While decoupling DisposeScope from block scope, making it similar to Autorelease Pool Blocks in Objective-C. So we don't need explicit transfer of ownership when a function is using some Disposable resource but wants to delegate its disposal to a parent DisposeScope. Also a DisposeScope can be stored and re-opened to support use cases like class.

So it would look something like this:

interface DisposeScope {
    defer<T>((value: T) => Promise<void>|void): void;
}

interface DisposableFunction<T, Args extends any[]> {
    (scope: DisposeScope, ...Args): T;
}

interface DisposableClass<T, Args extends any[]> {
    new (scope: DisposeScope, ...Args): T;
}

function Allocate(scope: DisposeScope, length: number): Array {
    const arr = new Array(length);
    scope.defer(() => arr.splice(0, arr.length));
    return arr;
}

function Bar() {
    const bar = using Allocate(10); // using always bind to the nearest dispose scope above the call stack
}

async dispose { // create a DisposeScope at this stack level
    const foo = using Allocate(5);
    {
        Bar();
    }
} // await bar dispose(); awiat foo dispose();

const baz = using Allocate(5); // error: no enclosing DisposeScope

// For class

class Resource {
    constructor(scope: DisposeScope) {
        this.#scope = scope;
    }
    async run(): Promise<void> {
        async dispose(this.#scope) { // reopen a DisposeScope
            this.#arr = using Allocate(5);
        } // disposal is being deferred to the owner of this.#scope
    }
}

async dispose { // create a DisposeScope at this stack level
    const resource = using new Resource();
} // await resource.#arr dispose();

Enclosing some code inside a DisposeScope means you want the scope to collect all the resource dispose methods, and call them when the DisposeScope is closed, and to make sure all dispose methods complete before executing the statements after the DisposeScope block.

You can even take it further to not pass DisposeScope as an argument. Instead making it an environment slot on the execution context. A defer keyword can be added to register a disposal method to the enclosing DisposeScope. However storing and re-opening a scope would require some changes to be supported.

So it can even look something like this:

function Allocate(length: number): Array {
    const arr = new Array(length);
    defer arr.splice(0, arr.length); // the expression is deferred for evaluation in the enclosing DisposeScope
    async defer { // a block expression can be used for defer
        await cleanup(); // await can be used after async defer
    }; // async defer can be used in a sync function as long as the enclosing DisposeScope is async dispose
    return arr;
}

function Bar() {
    const bar = Allocate(10); // function call always inherits the nearest dispose scope above the call stack
}

async dispose { // create a DisposeScope at this stack level
    const foo = Allocate(5);
    {
        Bar();
    }
} // await bar dispose(); awiat foo dispose();

const baz = Allocate(5); // error: no enclosing DisposeScope

// For class

function EnclosingDisposeScope(): unique symbol {
    // [native function]
    // returns a special symbol uniquely identifies the current enclosing DisposeScope
}

class Task {
    constructor() {
        this.#scope = EnclosingDisposeScope();
    }
    run(): void {
        dispose(this.#scope) { // reopen a DisposeScope identified by the symbol
            this.#arr = Allocate(5);
        } // disposal is being deferred to the scope identified by this.#scope
    }
}

async dispose { // create a DisposeScope at this stack level
    const task = new Task();
    dispose { // a new DisposeScope at top of the stack
        task.run();
    }
} // await task.#arr dispose();

In this way there's no need to define a Disposable type, and no need to use a DisposableStack. Even the using keyword can be omitted. A function created disposable resource no longer has to return a Disposable type. Every function just return whatever value they produce, and defer statement can be inserted at any location to dispose resource at the enclosing DisposeScope.

In this way, a disposal cannot be scheduled with defer if there's no enclosing DisposeScope. And the only way to register disposal operation is to use defer, and the dispose method is never exposed to the outside.

This would provide a safer default behavior when using Disposable resource. If you somehow "forgot" to create an new DisposeScope, resource created directly or indirectly from your function won't leak silently - it either fails to defer to an enclosing DisposeScope, resulting an error being thrown, or it's guarenteed to be disposed when the enclosing DisposeScope closed. If a resource is living longer than you expected, it's very likely being deferred to some long running DisposeScope, and you can easily find both the DisposeScope and the dispose method it referecnes in a heap dump. Diagnosing resource retaintion problems would be easier this way.

Extending the lifetime of a DisposeScope

Resource lifetime and execution context (functions, try-catch blocks etc) lifetime are loosely related. Resource references can be cloned, assigned, transferred, returned, down and up the call stack. You simply cannot confine a resouce in a fixed block of code enclosed by a pair of curly braces. Execution context on the other hand, is more ordered - it's always in a Last-In-First-Out stack. There are special behaviors like async/await, macrotasks, generator functions but if you generalize execution context enough, these can also follow the same rule.

A curly brace block syntax for a resource DisposeScope made it easy to implicitly pass the scope down the execution stack, but it doesn't capture the dynamic nature of resource passing between different execution contexts, and in many cases, outside the curly brace block the DisposeScope can enclose. When that happen, we will have use-after-free problems:

function createHandle(): Handle {
    const { write, close } = nativeHandle();
    defer close();
    return { write };
}
dispose {
    const handle = createHandle();
    doSomething().then(() => {
        handle.write(data); // error: handle already disposed
    }); // this returned synchronously while the callback is scheduled on an async context
}

Note that DisposableStack also has this problem:

function createHandle(): Disposable & Handle {
    const { write, close } = nativeHandle();
    return { write, [Symbol.dispose](): { close() } };
}
function main() {
    using handle = createHandle();
    doSomething().then(() => {
        handle.write(data); // error: handle already disposed
    }); // this returned synchronously while the callback is scheduled on an async context
}

However, if we decouple the DisposeScope from execution block scope, we can define a different semantic for it. Just look at the following code and it would be very intuitive if it can just work:

function createHandle(): Handle {
    const { write, close } = nativeHandle();
    defer close();
    return { write };
}
dispose { // spoiler: this should be an async dispose
    const scope = EnclosingDisposeScope();
    const handle = createHandle();
    doSomething().then(() => {
        dispose(scope) { // re-open a captured scope on an async context
            handle.write(data);
        }
    });
}

An immediate observation tell us, we can make it work if we add the following rule for a DisposeScope disposal: Only dispose a DisposeScope when all the references to that scope were gone (unreachable / garbage collected).

We can manually analyze the bahavior by denoting reference counts of the scope.

dispose { // open the scope, refcount++: 1
    const scope = EnclosingDisposeScope(); // refcount++: 2
    const handle = createHandle();
    doSomething().then(/* ... */); // captures scope, refcount++: 3
    // const scope gone, refcount--: 2
} // close the scope, refcount--: 1

// on the async execution context:
() => {
    dispose(scope) { // re-open the scope, refcount++: 2
        handle.write(data);
    } // close the scope, refcount--: 1
} // captured scope gone, refcount--: 0, now we can dispose scope

We can see that after reaching the end of the dispose block that created the DisposeScope, there is still reference to the scope. So the dispose doesn't happen. It's until the scheduled async context finished, and the reference count of the scope dropped to 0, we can finally dispose the scope.

This however, still has a problem. The DisposeScope we created here is an synchronous scope. But the refcount of the scope only reach to 0 on a different async execution context. This means we should use an async dispose scope instead, even the deferred function is synchronous:

function createHandle(): Handle {
    const { write, close } = nativeHandle();
    defer close();
    return { write };
}
async dispose {
    const scope = EnclosingDisposeScope();
    const handle = createHandle();
    doSomething().then(() => {
        async dispose(scope) { // re-open the captured scope on an async context
            handle.write(data);
        }
    });
}

We can write self managed resources:

function createHandle(): Handle {
    const scope = EnclosingDisposeScope();
    const { write, close } = nativeHandle();
    defer close();
    return { write, scope }; // extending the scope's lifetime to the lifetime of the returned object
}
async dispose {
    const handle = createHandle();
    doSomething().then(() => {
        // no need to capture the scope explicitly, handle will hold a reference to the scope already
        handle.write(data);
    });
}

// For class

class Task {
    constructor() {
        this.#scope = EnclosingDisposeScope(); // extending the scope's lifetime to the lifetime of the returned object
    }
    run(): void {
        dispose(this.#scope) {
            this.#arr = Allocate(5);
        }
    }
}
async dispose {
    const task = new Task();
    task.run();
}

We can even keep improving this by integrating async context tracking mechanism like Node.js AsyncLocalStorage / AsyncResource or the AsyncContext proposal. So the DisposeScope itself is an AsyncResource.

There are however some performance implications if we binds the lifetime of an async DisposeScope to its references. It means the dispose can only happen after a GC cycle. But personally I think the benifits are worth the tradeoff. We can achieve almost implicit usage of managed resource this way.

rixtox commented 1 year ago

I made a proof-of-concept preliminary implimentation based on Node.js AsyncLocalStorage / AsyncResource and FinalizationRegistry.

https://github.com/lyfejs/DisposeContext-PoC/blob/main/src/DisposeContext.test.ts

For the test code below:

function log(...args: any[]): void {
    const time = Math.floor(performance.now());
    console.log(time, ...args);
}

async function sleep(ms: number): Promise<void> {
    return new Promise(resolve => setTimeout(resolve, ms));
}

function Allocate(length: number): Array<any> {
    const arr = new Array(length).fill(length);
    DisposeContext.defer(() => log('deallocate end'));
    DisposeContext.deferAsync(async () => sleep(500));
    DisposeContext.defer(() => arr.splice(0, length));
    DisposeContext.defer(() => log('deallocate start'));
    return arr;
}

async function main() {
    log('main start');
    await DisposeContext.runAsync(async () => {
        log('AsyncDisposeContext scope start');
        const arr = Allocate(5);
        log('arr =', arr); // [5, 5, 5, 5, 5];

        setTimeout(() => {
            log('AsyncDisposeContext setTimeout start');
            log('arr =', arr); // [5, 5, 5, 5, 5];
            log('AsyncDisposeContext setTimeout end');
        }, 1000);

        log('AsyncDisposeContext scope end');
    });
    log('main end');
    exit(0);
}

if (typeof global.gc === 'function') {
    // force using a more frequent gc cycle (--expose-gc flag required)
    setInterval(() => {
        log('gc');
        gc!();
    }, 2000);
} else {
    // this sleep is required to keep the macrotask queue non-empty
    // otherwise when we wait for gc our microtask queue would be empty, and
    // the process would be terminated, before gc can kick-in
    sleep(60 * 1000);
}

main();

We can get the following output:

704 main start
705 AsyncDisposeContext scope start
706 arr = [ 5, 5, 5, 5, 5 ]
707 AsyncDisposeContext scope end
1709 AsyncDisposeContext setTimeout start
1710 arr = [ 5, 5, 5, 5, 5 ]
1711 AsyncDisposeContext setTimeout end
2717 gc
2730 deallocate start
3243 deallocate end
3245 main end

Note that the resource registered to the async DisposeContext doesn't start deallocation until all the async context captured the DisposeContext are finished. The main function only resumes after resource deallocation finish.

Also note that the resource returned by Allocate doesn't hold a reference to the enclosing DisposeContext but still the async setTimeout callback is able to extend the lifetime of the resource. This is achieved using AsyncLocalStorage. If you need to pass resource to other async context, you can make the resource subclassing AsyncContext and it would automatically capture the async context it was created on. For example:

class Foo extends AsyncResource {
    constructor(public value: string) {
        super('Foo');
        DisposeContext.defer(() => log('foo deallocate end'));
        DisposeContext.deferAsync(async () => sleep(500));
        DisposeContext.defer(() => log('foo deallocate start'));
    }
}

async function main() {
    log('main start');
    await DisposeContext.runAsync(async () => {
        log('AsyncDisposeContext scope start');
        const arr = Allocate(5);
        log('arr =', arr); // [5, 5, 5, 5, 5];
        let foo = new Foo('hello');

        setTimeout(() => {
            log('AsyncDisposeContext setTimeout 500 start');
            log('arr =', arr); // [5, 5, 5, 5, 5];
            log('AsyncDisposeContext setTimeout 500 end');
        }, 500);

        DisposeContext.runAsync(async () => {
            setTimeout(() => foo.runInAsyncScope(() => {
                log('AsyncDisposeContext setTimeout 1000 start');
                log('foo =', foo.value);
                log('AsyncDisposeContext setTimeout 1000 end');
            }), 1000);
        });

        log('AsyncDisposeContext scope end');
    });
    log('main end');
    exit(0);
}

This would output:

641 main start
642 AsyncDisposeContext scope start
643 arr = [ 5, 5, 5, 5, 5 ]
644 AsyncDisposeContext scope end
1148 AsyncDisposeContext setTimeout 500 start
1148 arr = [ 5, 5, 5, 5, 5 ]
1148 AsyncDisposeContext setTimeout 500 end
1658 AsyncDisposeContext setTimeout 1000 start
1659 foo = hello
1659 AsyncDisposeContext setTimeout 1000 end
2650 gc
4659 gc
4672 foo deallocate start
5187 foo deallocate end
5187 arr deallocate start
5697 arr deallocate end
5699 main end

Notice that even we enclosed second setTimeoue inside a new DisposeContext, we can reopen the async context of where foo was created by foo.runInAsyncScope(). This would extend the lifetime of the outmost DisposeContext to cover both the setTimeout callbacks.

mhofman commented 1 year ago

I'm not sure I follow what the suggestion is. The dispose mechanism is about explicit resource management, where the resource is explicitly owned and deterministically disposed of when exiting a scope.

You can already perform some implicit cleanup through FinalizationRegistry, however that is neither guaranteed to execute, nor deterministic as anything that is relying on garbage collection.

In my mind, those 2 things are completely different type of problems, with different use cases. I don't believe we should rely on garbage collection mechanisms to controls the disposal of held resources.

mhofman commented 1 year ago

IMO forgetting to declare a disposable with using is similar to not awaiting an async function. There are type based linting rules that can help catch those mistakes, but I don't see how a dynamic language like JS could enforce anything (unless you somehow could shoehorn linear typing into the language, which you can't).

rixtox commented 1 year ago

I'm not sure I follow what the suggestion is. The dispose mechanism is about explicit resource management, where the resource is explicitly owned and deterministically disposed of when exiting a scope.

I think we should not focus on whether the resource management is implicit or explicit, but rather think in terms of ownership. I believe ownership should be declared, not assigned. When you have implicit resource management, a resource ownership is declared when you initialize that variable. Assigning a variable to another is declaring the extension of the resource ownership lifetime to the other variable.

This is not true for the current Disposable approach. You can declare a variable but not owning its resource.

You can already perform some implicit cleanup through FinalizationRegistry, however that is neither guaranteed to execute, nor deterministic as anything that is relying on garbage collection.

In my mind, those 2 things are completely different type of problems, with different use cases. I don't believe we should rely on garbage collection mechanisms to controls the disposal of held resources.

That's fine. We don't have to leverage garbage collection to trigger disposal. I was just outlining what are possible for making resource ownership to be declarative rather than imperative.

IMO forgetting to declare a disposable with using is similar to not awaiting an async function

Not awaiting an async function won't leak - it's still scheduled to be run on task queue, you are just not joining with the async completion to your current execution context. And there are legit use cases for not awaiting for an async function.

But not using a Disposable has serious consequence - it leaks quietly even after it being garbage collected. I cannot think of a single legit reason of not binding a Disposable resource to some scope.

And by the way there are more problems in this design - can you dispose a resource twice? If I'm authoring a Disposable object, should I keep track of if the dispose method has been called already and produce an error or ignore subsequence release if someone is doing excess release? Can you bind a Disposable resource to multiple scopes/stacks? How can you unbind a resource? Of course these are all anti-patterns and misuses. But the current design is prone to these errors.

One thing I also proposed above is to not expose the dispose method as an interface. This would encourage misuses and anti-patterns to abuse it to do things it was not intended for. Like in #156 and #158. Instead bind the disposal directly to some scope would address both my concerns above. And you don't need a linter to enforce that. This can all be done with a proper design.

mhofman commented 1 year ago

When you have implicit resource management, a resource ownership is declared when you initialize that variable. Assigning a variable to another is declaring the extension of the resource ownership lifetime to the other variable.

I think this is where I disagree. Assignment is not the same as gaining ownership.

As I mentioned, JS cannot have a notion of ownership through assignment, which is basically linear types.

The using mechanism is the way to declare ownership. And indeed nothing prevents you from having multiple code paths independently claiming ownership of an object, or none at all, as that is basically impossible to add to the JS language at this point.

rixtox commented 1 year ago

I think this is where I disagree. Assignment is not the same as gaining ownership.

As I mentioned, JS cannot have a notion of ownership through assignment, which is basically linear types.

In a garbage collection based language like JavaScript, you are right there's no explicit notion of ownership. But objects and resource still have their lifetime. An object's lifetime starts at when it's been declared/initialized, and ends when it's no longer reachable from the root. So in a very generalized way, we can say in JavaScript everything is owned by the root variable context. You extend the lifetime of that ownership when you assign the object to another variable.

Of course JavaScript would never become linear typed like Rust. And I don't expect that to happen. But I do expect a resource management design to be safer under general usage, and can prevent common mistakes in normal use cases.

The using mechanism is the way to declare ownership.

The using mechanism does not "declare" an ownership. It assigns an ownership. IMO this mechanism is flawed mainly because of 2 reasons:

  1. An easy mistake of not using a Disposable would cause dangerous leak - this is just unacceptable. There's no justification of it being allowed. I made it the title of this issue for a reason. It's the biggest problem of the current proposal. It's so dangerous and illegal to not using a Disposable, much more damaging than not awaiting an async function. It's so serious that I believe the language has to prevent it from happening at all. We cannot allow it to happen and just tell people to use a linter to catch the mistake. And the solution to this issue is actually not that hard - the language should enforce the binding of a resource to a resource scope enclosing the resource creation, and throw an error if there's no such scope.

  2. using is binding resource to the block scope. Block scopes are part of execution context. I mentioned it couple times but let me empathize: Execution context and resource context should not be considered as the same thing and should be decoupled in program design. Execution context are generally stack based. Even async callbacks are scheduled with push and pop operations on some queue. You can correlate microtasks with some stack based context. This is what AsyncContext has been trying to make explicit of. However resource context is completely different. Resource can be passed around different levels of execution context and being shared, making them not bound to a single execution stack frame. Binding a resource only to a block scope, which is part of the execution context, is not that useful. You may want to return the resource. You may want to capture the resource in a closure. You may want to assign it to an outer scope or on some global data structures. Yes there's DisposableStack but the stack itself is a Disposable and itself has the same problem. The solution to this issue is also the same - just bind a resource to a resource scope, not an execution context scope. If you read my proposed possible solutions in the main issue article, you would understand how the decoupling of resource context and execution context actually work. This is already a thing in language like Objective-C where it has @autorelease {} blocks being a decoupled resource context than the execution context.

Although execution context and resource context should be decoupled, they need to work together to make sure correct program behavior:

  1. The execution context that created the resource context must join with (await on) that resource context to be collected (deallocated/disposed).
  2. A resource context may enclose multiple execution context and capture resources created during the execution.
  3. An execution context referencing a resource from outside of that execution context, should extend the lifetime of that resource's binding resource context to the end of the current execution context. Or use after free can happen.
  4. A resource is owned by its enclosing resource context. This ownership was established when the resource was declared, and cannot be assigned or changed. This achieves declarative ownership.
  5. You can safely pass the resource to different execution contexts - calling a function, making a closure as callback etc, as long as that execution context is enclosed by the resource's binding resource context.
bakkot commented 1 year ago

The approach in this proposal is similar to the approach in other languages like Python and Java, where it works fine. Yes, it's possible for a resource to get leaked, but that is inevitable because even in the presence of a perfect resource management system there would be no requirement to use it with any of the many resources which exist today. It is extremely unlikely that the committee is going to choose a different approach, especially at this late stage.

Separately, in my opinion the approach you suggest is significantly worse than the current approach in terms of ability to reason about code. Resources should get cleaned up at a predictable point, not based on what other stuff happens to be on the callstack. And it's important that users be able to create and explicitly clean up long-lived resources, which might outlive the callstack; with the approach you've outlined the only way I can see to do that is a reachability analysis of the "resource context", which is not viable, or by letting users manually manage disposal of "resource context"s, which is equivalent to the current approach.

rixtox commented 1 year ago

It is extremely unlikely that the committee is going to choose a different approach, especially at this late stage.

That's very unfortunate. I also notice Ben Lesh, author of RxJS, also only discovered this proposal very recently, and he also expressed his concerns over different reasons. It would be a shame if an important language feature like this cannot have a chance to address community feedbacks before it ships.

Resources should get cleaned up at a predictable point, not based on what other stuff happens to be on the callstack.

That is not true. This is the first thing I mentioned how resource context should work:

The execution context that created the resource context must join with (await on) that resource context to be collected (deallocated/disposed).

The deallocation happens when the binding resource context closes.

And it's important that users be able to create and explicitly clean up long-lived resources

You can do this by declaring the resource on a specific resource context that you have finer control of. This is the same how Objective-C is doing. Just read the Use Local Autorelease Pool Blocks to Reduce Peak Memory Footprint section.

the only way I can see to do that is a reachability analysis of the "resource context", which is not viable, or by letting users manually manage disposal of "resource context"s, which is equivalent to the current approach.

It can be done in very simple and intuitive way. If you want to extend the lifetime of a resource context to some async function for example, just wrap that async function. So a resource context would open before starting the async function, and close after the async function completes. This is already a pattern used in e.g. AsyncContext.run() where you can wrap function calls within an async context.

bakkot commented 1 year ago

The deallocation happens when the binding resource context closes.

I appreciate that this is a property your design has. It is a property that I specifically think we should not have, because what the "binding resource context" is, and when it closes, depends on the full state of the callstack at the time the resource is allocated, which is very hard to reason about.

It can be done in very simple and intuitive way. If you want to extend the lifetime of a resource context to some async function for example, just wrap that async function.

What does it mean to "extend the lifetime of a resource context to some async function"? Does that mean the context is kept alive until all references to that function have been garbage collected? Like I said, approaches depending on GC are not viable. Or do you mean that you're immediately invoking the function and the resource context is kept alive only for a single execution of the function? That doesn't cover most use cases, where the function may not be executed immediately.

Here's a common pattern: I create a resource, and later - for example, in an event listener - I use the resource, and then immediately dispose of it. What are you imagining that looks like with your design? Please write it out in sample code.

For reference, here's one way to do that with the design in the proposal:

let resource = getResource();
addEventListener('click', () => {
  using res = resource;
  res.use();
  // resource is disposed of here
}, { once: true });
rixtox commented 1 year ago

Here's a common pattern: I create a resource, and later - for example, in an event listener - I use the resource, and then immediately dispose of it. What are you imagining that looks like with your design? Please write it out in sample code.

For reference, here's one way to do that with the design in the proposal:

let resource = getResource();
addEventListener('click', () => {
  using res = resource;
  res.use();
  // resource is disposed of here
}, { once: true });

Without any syntax changes to ES6, I can do this:

await context.runAsync(async (context) => {
    const resource = getResource(context);
    context.runAsync(() => new Promise((resolve) => {
        addEventListener('click', () => {
            res.use();
            resolve();
        }, {once: true});
    }));
});
// resource is disposed of here

Where getResource also need to be aware of the context:

function getResource(context) {
    context.defer(() => teardown());
    // ...
}

Your code would fail in the following case:

let resource = getResource();
addEventListener('click', () => {
  using res = resource;
  res.use();
}, { once: true });

addEventListener('keydown', () => {
  using res = resource;
  res.use();
}, { once: true });

But my approach would still work:

await context.runAsync(async (context) => {
    const resource = getResource(context);
    context.runAsync(() => new Promise((resolve) => {
        addEventListener('click', () => {
            res.use();
            resolve();
        }, {once: true});
    }));
    context.runAsync(() => new Promise((resolve) => {
        addEventListener('keydown', () => {
            res.use();
            resolve();
        }, {once: true});
    }));
});
bakkot commented 1 year ago

In that example, if you fail to call resolve - for example, if res.use() throws - then your resource will leak forever. This is exactly the same problem you're worried about with the current proposal. (In fact it's worse, since now you're also leaking arbitrarily many other resources which happened to be declared in the same context.)

So the approach you're suggesting doesn't actually improve anything over the current proposal: it still requires discipline on the part of developers to ensure they're using the appropriate patterns to handle resources (or resource scopes). And it's also significantly more complex. So it seems strictly worse than the current proposal.

mhofman commented 1 year ago

Also this approach is bound to the context object not being stored somewhere, which would allow whoever has a hold of the context object to perform another runAsync, extending the context's life. More broadly, AsyncContext is not yet a language feature, and there are ongoing discussions about what it means for a context to terminate (spoiler alert, I believe it is intrinsically tied to garbage collection).

rixtox commented 1 year ago

No it’s not worse. Yes if your promise never resolves, sure your program may hang. But the uncaught error here will be visible to developers and actionable. Unlike the silent leak of not using a resource.

bakkot commented 1 year ago

An exception in an event listener is not the only way to fail to call resolve. In that specific example you might get an actionable error somewhere, but if you happened to have a try/catch around listener, or just had a branch where you omitted resolve, you would not.

rixtox commented 1 year ago

If you try to run on a disposed context of course an error will be thrown. That’s the best we can provide so far. It’s the same as returning a resource that you bind to the current scope with using. Without object level or context level reference tracking this use case can never be supported. But this is not a feature I really need for now.

rixtox commented 1 year ago

An exception in an event listener is not the only way to fail to call resolve. In that specific example you might get an actionable error somewhere, but if you happened to have a try/catch around listener, or just had a branch where you omitted resolve, you would not.

Of course to support any language feature, some disciplines need to be followed. In this case you need to make sure your asynchronous function completes properly. I think that’s a very reasonable ask.

You don’t need to await on the asynchronous function that you wrapped to run on the context though. The context can await it for you.

rixtox commented 1 year ago

Even in the worst case where you made a mistake and your Promise doesn’t complete and leaking the resource, it visible in the heap dump, with all retain relations available to the developer.

While with using, your resource can disappear from heap dump when leaked, leaving you no trace to diagnose.

Which one is worst?

bakkot commented 1 year ago

In this case you need to make sure your asynchronous function completes properly. I think that’s a very reasonable ask.

Since the asynchronous function involved is necessary resolved only by an explicit call to resolve, that seems like very much the same kind of thing as "resources should be consumed using using", except that the latter is much easier for static analysis to provide errors for. If you're ok with requiring that level of discipline from developers, then the current proposal is fine.

While with using, your resource can disappear from heap dump when leaked, leaving you no trace to diagnose.

I do not think enough developers currently or will ever make use of heap dumps for it to make sense to design a feature on the basis of how it will affect a heap dump.

rixtox commented 1 year ago

One main reason for having any resource management mechanism is to address leaks. And a heap dump is the ad hoc thing to try when diagnosing leaks. It would be outrageous if a resource management proposal doesn’t take leak diagnosis into consideration.

Just to ask the reverse, if your big code base had a ghost leak not showing in heap dump, what else can you do?

You don’t want to later tell people heap dump is not something common developers use so you didn’t bother to support it when designing the Disposable.

rbuckton commented 1 year ago

I'm a bit uneasy with the fact that it's allowed to create a Disposable without registering its dispose method to some kind of disposal scope, either with using on the current scope, or add to some DisposableStack.

In fact, the disposal registration is opt-in, not opt-out. This gives a Disposable object leaky behavior by default. It's like still doing malloc() and free(), but added opt-in support for scope based RAII. This is especially concerning given most of the resource management in JavaScript to date has been implicit by garbage collection. A Disposable object can leak even after it getting garbage collected.

function createHandle(): Disposable & Handle {
    const { write, close } = nativeHandle();
    return { write, [Symbol.dispose](): { close() } };
}

function main() {
    const handle = createHandle();
    handle.write(data);
    // ops you forget to dispose handle
}

main();
// now it's leaking quietly
// any because handle is garbage collected you can't even find it in heap dump

It's worse than leaking - it's leaking quietly, and it can disappear from heap dump because the dispose method can be garbage collected. Diagnosing a leak like this would be a nightmare.

In a language like C#, this is often handled through the use of a finalizer to guarantee cleanup on GC:

public class ManagedHandle : Disposable
{
    private IntPtr handle;

    public ManagedHandle()
    {
        handle = GetNativeHandle();
    }

    ~ManagedHandle()
    {
        // We're doing cleanup implicitly, so just release the handle.
        ReleaseNativeHandle(this.handle);
    }

    public void Dispose()
    {
        // We're doing cleanup explicitly, so suppress the finalizer for this object
        // and release the native handle
        GC.SuppressFinalize(this);
        ReleaseNativeHandle(this.handle);
        this.handle = IntPtr.Zero;
    }
}

The same can be accomplished in JS with a FinalizationRegistry:

const managedHandleRegistry = new FinalizationRegistry(close => {
    // We're doing cleanup implicitly, so just release the handle.
    close();
});
function createHandle() {
    const { write, close } = nativeHandle();
    const unregisterToken = {};
    const managedHandle = {
        write,
        [Symbol.dispose]() {
            // We're doing cleanup explicitly, so suppress the finalizer for this object
            // and release the native handle
            managedHandleRegistry.unregister(unregisterToken);
            close();
        }
    };
    managedHandleRegistry.register(managedHandle, close, unregisterToken);
    return managedHandle;
}

function main() {
    const handle = createHandle();
    handle.write(data);
    // oops, forgot to dispose handle
}

main();
// no leak because the cleanup callback will trigger if we didn't explicitly dispose.

So it would look something like this:

[...]

In this way there's no need to define a Disposable type, and no need to use a DisposableStack. Even the using keyword can be omitted. A function created disposable resource no longer has to return a Disposable type. Every function just return whatever value they produce, and defer statement can be inserted at any location to dispose resource at the enclosing DisposeScope.

In this way, a disposal cannot be scheduled with defer if there's no enclosing DisposeScope. And the only way to register disposal operation is to use defer, and the dispose method is never exposed to the outside.

While I understand its the point of your proposal, I feel that the design is far too rigid. This makes it signifcantly harder to manually manage lifetime of an object if it has to be tied to some outer dispose scope if an object's lifetime needs to extend past any reasonable block scope. The only solution to that in your proposal is to reopen a scope and somehow leave it dangling, possibly with a generator or async function that becomes suspended, and that is far too esoteric of a solution.

This would provide a safer default behavior when using Disposable resource. If you somehow "forgot" to create an new DisposeScope, resource created directly or indirectly from your function won't leak silently - it either fails to defer to an enclosing DisposeScope, resulting an error being thrown, or it's guarenteed to be disposed when the enclosing DisposeScope closed. If a resource is living longer than you expected, it's very likely being deferred to some long running DisposeScope, and you can easily find both the DisposeScope and the dispose method it referecnes in a heap dump. Diagnosing resource retaintion problems would be easier this way.

This can already be accomplished with FinalizationRegistry, as I've shown above. While using a FinalizationRegistry is an opt-in behavior, its also a lot of overhead that most resources won't need. Instead, I think its better to opt-in to the additional overhead where warranted, rather than force it on all users. In addition, FinalizationRegistry was designed in such a way as to avoid holding onto finalizable object reference, which would itself either prevent finalization or require more complexity to track object ressurection. I'll get into more about why that's important in my response to "Extending the lifetime of a DisposeScope", below.

Extending the lifetime of a DisposeScope

Resource lifetime and execution context (functions, try-catch blocks etc) lifetime are loosely related. Resource references can be cloned, assigned, transferred, returned, down and up the call stack. You simply cannot confine a resouce in a fixed block of code enclosed by a pair of curly braces. Execution context on the other hand, is more ordered - it's always in a Last-In-First-Out stack. There are special behaviors like async/await, macrotasks, generator functions but if you generalize execution context enough, these can also follow the same rule.

A curly brace block syntax for a resource DisposeScope made it easy to implicitly pass the scope down the execution stack, but it doesn't capture the dynamic nature of resource passing between different execution contexts, and in many cases, outside the curly brace block the DisposeScope can enclose. When that happen, we will have use-after-free problems:

function createHandle(): Handle {
    const { write, close } = nativeHandle();
    defer close();
    return { write };
}
dispose {
    const handle = createHandle();
    doSomething().then(() => {
        handle.write(data); // error: handle already disposed
    }); // this returned synchronously while the callback is scheduled on an async context
}

Note that DisposableStack also has this problem:

function createHandle(): Disposable & Handle {
    const { write, close } = nativeHandle();
    return { write, [Symbol.dispose](): { close() } };
}
function main() {
    using handle = createHandle();
    doSomething().then(() => {
        handle.write(data); // error: handle already disposed
    }); // this returned synchronously while the callback is scheduled on an async context
}

I would argue that in this case, your explicit lifetime scope may not match the intended lifetime, and should be rewritten:

async function main() {
    using handle = createHandle();
    await doSomething();
    handle.write(data);
}

If your intent was that handle shouldn't survive the async operation, then you would expect to get an error in the callback. If you need it to survive long enough for the callback to execute, then you would need to either change the lifetime scope (by using await), or switch to a slightly more imperative style:

function main() {
    using stack = new DisposableStack();
    const handle = stack.use(createHandle());
    const doSomethingPromise = doSomething();
    const saved = stack.move();
    doSomethingPromise.then(() => {
        // restore outer stack
        using stack = saved;
        handle.write(data);
    }, (e) => {
        // restore outer stack
        using stack = saved;
        throw e;
    });
}

However, if we decouple the DisposeScope from execution block scope, we can define a different semantic for it. Just look at the following code and it would be very intuitive if it can just work:

function createHandle(): Handle {
    const { write, close } = nativeHandle();
    defer close();
    return { write };
}
dispose { // spoiler: this should be an async dispose
    const scope = EnclosingDisposeScope();
    const handle = createHandle();
    doSomething().then(() => {
        dispose(scope) { // re-open a captured scope on an async context
            handle.write(data);
        }
    });
}

This is my major concern for your proposal, and the reason I brought up FinalizationRegistry earlier. As soon as you introduce scope capturing of an implicit surrounding scope, you have opened yourself up to memory and GC issues. If any callee can capture the implicit outer scope and save it, all of those resources lifetimes are no longer obvious to the caller. Since that scope can be restored at any time it can never be freed if any of the closures that capture it are still alive. And this capturing mechanism would need to keep alive all of the outer implicit scopes since there is no telling whether an inner resource has an ordered disposal dependency on an outer resource.

The goal of DisposableStack was to give you the kind capability you are suggesting here, but make it far more explicit how that lifetime is managed and extended. Implicit scoping makes it far less obvious what the impact will be.

An immediate observation tell us, we can make it work if we add the following rule for a DisposeScope disposal: Only dispose a DisposeScope when all the references to that scope were gone (unreachable / garbage collected).

We can manually analyze the bahavior by denoting reference counts of the scope.

dispose { // open the scope, refcount++: 1
    const scope = EnclosingDisposeScope(); // refcount++: 2
    const handle = createHandle();
    doSomething().then(/* ... */); // captures scope, refcount++: 3
    // const scope gone, refcount--: 2
} // close the scope, refcount--: 1

// on the async execution context:
() => {
    dispose(scope) { // re-open the scope, refcount++: 2
        handle.write(data);
    } // close the scope, refcount--: 1
} // captured scope gone, refcount--: 0, now we can dispose scope

We can see that after reaching the end of the dispose block that created the DisposeScope, there is still reference to the scope. So the dispose doesn't happen. It's until the scheduled async context finished, and the reference count of the scope dropped to 0, we can finally dispose the scope.

Ref counting isn't nearly as reliable here as you make it out to be. You can't count references statically because scope, or the arrow function capturing it, can be held in memory. Instead, you must wait until scope is garbage collected to decrement the reference. At that point, you are better off using FinalizationRegistry.

This however, still has a problem. The DisposeScope we created here is an synchronous scope. But the refcount of the scope only reach to 0 on a different async execution context. This means we should use an async dispose scope instead, even the deferred function is synchronous: [...]

There's no point in ref counting if the scope still can't survive past the dispose {} block it captures.

We can even keep improving this by integrating async context tracking mechanism like Node.js AsyncLocalStorage / AsyncResource or the AsyncContext proposal. So the DisposeScope itself is an AsyncResource.

Async context tracking is already a Stage 2 proposal. While I am generally supportive of that proposal, I'm strongly against language level implicit passdown of disposable scopes.

There are however some performance implications if we binds the lifetime of an async DisposeScope to its references. It means the dispose can only happen after a GC cycle. But personally I think the benifits are worth the tradeoff. We can achieve almost implicit usage of managed resource this way.

If dispose can only happen after GC, then your proposal does not give you the ability to enforce lifetime with timely cleanup. One of the main motivations of this proposal was to have an explicitly bounded lifetime for a resource that allow you to avoid race conditions with file IO and thread synchronization. Implicit scoping and capture would make those scenarios impossible, since you could never have an explicitly bounded lifetime.

rixtox commented 1 year ago

@rbuckton First of all, I'd like to thank you for taking time to understand my concerns and provide a through response. I really appreciate your professional attitude. It would be better if stakeholders like RxJS, Angular, and other popular frameworks have been asked for feedback in early stages of the proposal. Important changes like this should get more attention from the developer community.

Apologize that I expressed several concerns mixing together. Making it harder to address them one by one. So let me separate them in the order of importance. And allow me to reorder your response into each category and address them with clear context and intent.

1. Leak diagnose

There should always be a retain path from the VM root to a living resource, or the teardown functions of that living resource. The current design made it easy to lose that info when a leak happens. This is not asking for preventing leaks from happening. This is an ask to provide the useful and actionable information developers need when there's a leak happening. Leaks are generally quiet by nature so the retain path is the most essential info for diagnosing leaks. A resource management mechanism should preserve the retain path info as much as possible. It's the bottom line for not being able to fully prevent leaks.

This can already be accomplished with FinalizationRegistry, as I've shown above. While using a FinalizationRegistry is an opt-in behavior, it's also a lot of overhead that most resources won't need. Instead, I think its better to opt-in to the additional overhead where warranted, rather than force it on all users.

I know I was the one first mentioned about FinalizationRegistry, but under a different context. Of course if a Disposable incorporated FinalizationRegistry to guarantee its finalization correctly like you showed above, it won't leak even not using it. But just like you said, this GC finalization behavior preventing leaks from happening should be opt-in. I completely agree, but I don't think it's something even experienced developers would want to use. However, as I emphasized, preventing leaks was never my intention.

Your response still doesn't address the undiagnosable nature of leaking by not using. We should not rely on linter to catch it. Linters can fail in may cases. I believe this can be done with a better language feature design.

2. Sharing resource to multiple detached async calls

I didn't state this issue in a clear sentence before. But one example I wrote above can demonstrate this issue. Let me adapt that example to show it more clear:

async function main() {
    using handle = createHandle();
    const data = await handle.readAsync();
    doSomething(data).then(async (newData) => {
        await handle.writeAsync(newData); // how to extend the handle's lifetime to here?
    });
    doSomethingElse(data).then(async (newData) => {
        await handle.writeAsync(newData); // how to extend the handle's lifetime to here?
    });
    return data;
}

I would argue that in this case, you're explicit lifetime scope may not match the intended lifetime... If you need it to survive long enough for the callback to execute, then you would need to either change the lifetime scope (by using await), or switch to a slightly more imperative style.

Calling async functions in detached mode (without using await) has legit use cases. The example above shows one of those cases. You want to read from the resource and return the result immediately, while scheduling some write operations to the resource without explicitly waiting for them to complete.

It is possible to do it imperatively with using, moving and restoring DisposableStacks around. But very soon you will find yourself decoupling resource context from execution context, reinventing resource context capturing in some way.

async function main() {
    using stack = new DisposableStack();
    const handle = stack.use(createHandle());
    const data = await handle.readAsync();
    const saved = stack.move();
    (async () => { // this is a scope created solely for extending resource lifetime
        using stack = saved;
        await Promise.allSettled([
            doSomething(data).then(async (newData) => {
                await handle.writeAsync(newData);
            }),
            doSomethingElse(data).then(async (newData) => {
                await handle.writeAsync(newData);
            }),
        ]);
    })();
    return data;
}

I don't think you can achieve this without making a dedicated resource context scope. Comparing to the other approach that separated resource context from execution context from the beginning, which is more intuitive to use:

async function main(context) {
    const handle = createHandle(context);
    const data = await handle.readAsync();
    context.runAsync(() => doSomething(data).then(async (newData) => {
        await handle.writeAsync(newData);
    }));
    context.runAsync(() => doSomethingElse(data).then(async (newData) => {
        await handle.writeAsync(newData);
    }));
    return data;
}

You can easily and intuitively extend resource context lifetime by running your async functions on it, in a declarative style.

3. Passing resource out of block scope without binding to some outer scope

While I understand it's the point of your proposal, I feel that the design is far too rigid. This makes it signifcantly harder to manually manage lifetime of an object if it has to be tied to some outer dispose scope if an object's lifetime needs to extend past any reasonable block scope.

I think you got a dilemma here:

  1. You don't want to force resources to be tied to a dispose scope at creation, so they can be passed around freely. Essentially making them dangling resources easy to be leaked.
  2. Leak diagnosis is still the biggest problem unsolved for not binding resources at creation.

You cannot have both for a language like JavaScript. So the question comes down to which is the right choice.

My reasoning to this problem is, at the end of the day, you still need to ensure your program is safe and sound. You can either make the language easy to write unsafe code, but hard to diagnose when developer made a mistake; or you can make the language encouraging developers to follow good practices, and provide useful diagnose support when they do make mistakes, while making it hard to write unsafe code.

To show you what I meant by this, and you probably already realized, it's possible to convert the other approach into explicit imperitive style:

// THIS IS UNSAFE
function unsafeRetain(context) {
    let release;
    context.runAsync(new Promise(resolve => release = resolve));
    return release;
}

Now you can pass the release method freely - but unsafe. It's now functioning in the same way as Disposable - without the using syntax etc. It can leak too if you forget to call release(). But it doesn't disappear from heap dump. This is the main difference from bare Disposable - it still has the context awaiting for the dispose to happen so it preserves the retain path for diagnosis.

that is far too esoteric of a solution

It's harder to turn it into this form but for a good reason - it's unsafe to do so. But you can gain that level of freedom if you know what you were doing.

4. Resource context capturing

I avoided using the dispose {} syntax so far because I wanted to seperate my context and intents. I want to dedicate this section for this topic.

The approach I proposed defined a contract to pass the resource context down the call stack. This has been done by explictly passing the context as function argument so far, but if we make it a language feature it may get a dedicated syntax.

As soon as you introduce scope capturing of an implicit surrounding scope, you have opened yourself up to memory and GC issues. If any callee can capture the implicit outer scope and save it, all of those resources lifetimes are no longer obvious to the caller.

As I mentioned above, although it's doable, it should be deemed unsafe and discoraged, unless you know what you are doing. This is at worst leaking with retain path info available. If you have nested DisposableStack binded together and you forgot to release the root stack, you leak everything on it too, without retain path info available.

And this capturing mechanism would need to keep alive all of the outer implicit scopes since there is no telling whether an inner resource has an ordered disposal dependency on an outer resource.

Yes. This is the same way if you constructed a nested DisposableStack, or have nested function calls each binded resources with using. This is the natural way nested resource dependencies should behave when the are released. You can break this pattern using unsafe methods of course, but we should discourage that.

4.1. Async resource context capturing and GC

I think this is the part most of the rest of your comments were about, and I agree with most of the things you said. Becuase myself also don't think we should leverage any GC features in a resource management mechanism. The intention of me bringing it up is to explore if it's possible to capture resource context across async executon contexts. I only said it's possible if GC is used, but I don't think it's a good idea to involve GC for any real implementation. I would be interested if there's another way to do it.

rbuckton commented 1 year ago

1. Leak diagnose

There should always be a retain path from the VM root to a living resource, or the teardown functions of that living resource. The current design made it easy to lose that info when a leak happens. This is not asking for preventing leaks from happening. This is an ask to provide the useful and actionable information developers need when there's a leak happening. Leaks are generally quiet by nature so the retain path is the most essential info for diagnosing leaks. A resource management mechanism should preserve the retain path info as much as possible. It's the bottom line for not being able to fully prevent leaks.

This can already be accomplished with FinalizationRegistry, as I've shown above. While using a FinalizationRegistry is an opt-in behavior, it's also a lot of overhead that most resources won't need. Instead, I think its better to opt-in to the additional overhead where warranted, rather than force it on all users.

I know I was the one first mentioned about FinalizationRegistry, but under a different context. Of course if a Disposable incorporated FinalizationRegistry to guarantee its finalization correctly like you showed above, it won't leak even not using it. But just like you said, this GC finalization behavior preventing leaks from happening should be opt-in. I completely agree, but I don't think it's something even experienced developers would want to use. However, as I emphasized, preventing leaks was never my intention.

I believe that runtimes will choose to adopt Disposable wrappers for most native handles, such as the NodeJS fs.promises.FileHandle, and they could easily leverage FinalizationRegistry for those wrappers. Runtimes may even choose to expose some kind of native handle wrapper as well:

interface SafeHandleData<T> {
    unsafeHandle: T;
    cleanup: (unsafeHandle: T) => void;
}

class SafeHandle<T> {
    static #dispose = ({ unsafeHandle, cleanup }: SafeHandleData<T>) => cleanup(unsafeHandle);
    static #registry = new FinalizationRegistry(SafeHandle.#dispose);
    #unregisterToken = {};
    #data: SafeHandleData<T> | undefined;

    constructor(unsafeHandle: T, cleanup: (unsafeHandle: T) => void) {
        this.#data = { unsafeHandle, cleanup };
        SafeHandle.#registry.register(this, this.#data, this.#unregisterToken);
    }

    get value() {
        if (!this.#data) throw new ReferenceError("Object is disposed");
        return this.#data.unsafeHandle;
    }

    dispose() {
        if (this.#data) {
            SafeHandle.#registry.unregister(this.#unregisterToken);
            const data = this.#data;
            this.#data = undefined;
            SafeHandle.#dispose(data);
        }
    }

    [Symbol.dispose]() {
        return this.dispose();
    }
}

It may even be worth a future add-on proposal to add something like SafeHandle natively.

If most native handles have Disposable wrappers, users won't need to reach for custom solutions, and thus those handles will either close on GC or appear in heap dumps. In other languages with this capability, the majority of user-written disposables are either some form of composition of existing disposables, or a disposable wrapper for some non-handle-based operation (such as tracing, transactions, etc.) that may not need such finalization behavior.

If all of the native resources you consume are already wrapped in some kind of safe handle wrapper, they will be readily visible in a heap snapshot.

Your response still doesn't address the undiagnosable nature of leaking by not using. We should not rely on linter to catch it. Linters can fail in may cases. I believe this can be done with a better language feature design.

Yes, leaks are possible with using. Yes, linters and type systems may not be able to be 100% reliable at catching this. However, I think this design is far more flexible than what you've suggested so far. Its far easier to write imperative code against Disposable when you explicitly don't want to leverage using. Its also far easier to integrate into the wealth of existing APIs without having to wholly restructure your code, which would otherwise be a severe adoption risk.

For example, I've been considering a follow-on proposal to add Symbol.dispose to ArrayBuffer to forcibly detach the array buffer so that its memory can be reclaimed. This proposal already adds [Symbol.dispose]() to built-in iterators that performs iter.return(), for cases where you need to execute an iterator imperatively rather than with for..of.

2. Sharing resource to multiple detached async calls

I didn't state this issue in a clear sentence before. But one example I wrote above can demonstrate this issue. Let me adapt that example to show it more clear:

async function main() {
    using handle = createHandle();
    const data = await handle.readAsync();
    doSomething(data).then(async (newData) => {
        await handle.writeAsync(newData); // how to extend the handle's lifetime to here?
    });
    doSomethingElse(data).then(async (newData) => {
        await handle.writeAsync(newData); // how to extend the handle's lifetime to here?
    });
    return data;
}

I would argue that in this case, you're explicit lifetime scope may not match the intended lifetime... If you need it to survive long enough for the callback to execute, then you would need to either change the lifetime scope (by using await), or switch to a slightly more imperative style.

Calling async functions in detached mode (without using await) has legit use cases. The example above shows one of those cases. You want to read from the resource and return the result immediately, while scheduling some write operations to the resource without explicitly waiting for them to complete.

It is possible to do it imperatively with using, moving and restoring DisposableStacks around. But very soon you will find yourself decoupling resource context from execution context, reinventing resource context capturing in some way.

async function main() {
    using stack = new DisposableStack();
    const handle = stack.use(createHandle());
    const data = await handle.readAsync();
    const saved = stack.move();
    (async () => { // this is a scope created solely for extending resource lifetime
        using stack = saved;
        await Promise.allSettled([
            doSomething(data).then(async (newData) => {
                await handle.writeAsync(newData);
            }),
            doSomethingElse(data).then(async (newData) => {
                await handle.writeAsync(newData);
            }),
        ]);
    })();
    return data;
}

I don't think you can achieve this without making a dedicated resource context scope. Comparing to the other approach that separated resource context from execution context from the beginning, which is more intuitive to use:

async function main(context) {
    const handle = createHandle(context);
    const data = await handle.readAsync();
    context.runAsync(() => doSomething(data).then(async (newData) => {
        await handle.writeAsync(newData);
    }));
    context.runAsync(() => doSomethingElse(data).then(async (newData) => {
        await handle.writeAsync(newData);
    }));
    return data;
}

You can easily and intuitively extend resource context lifetime by running your async functions on it, in a declarative style.

Intended resource lifetime can always vary based on need. using uses lexical block scoping to give you a clear boundary for a resource, while DisposableStack gives you both the ability to compose resources, as well as the ability to extend lifetime. If you want to have a resource lifetime that is associated with the execution context, you will already be able to do so programmatically with AsyncContext. But again, I am strongly opposed to natively supporting implicit pass down of disposable scopes as that approach is a GC hazard and can make lifetimes impossible to predict. That approach causes far more problems than it solves. If you wanted implicit pass down in your own code, you could do so with DisposableStack and it won't have any effect on using declarations in functions elsewhere in the call stack, so the authors of those functions can make reliable determinations about the lifetime of their resources.

3. Passing resource out of block scope without binding to some outer scope

While I understand it's the point of your proposal, I feel that the design is far too rigid. This makes it signifcantly harder to manually manage lifetime of an object if it has to be tied to some outer dispose scope if an object's lifetime needs to extend past any reasonable block scope.

I think you got a dilemma here:

  1. You don't want to force resources to be tied to a dispose scope at creation, so they can be passed around freely. Essentially making them dangling resources easy to be leaked.
  2. Leak diagnosis is still the biggest problem unsolved for not binding resources at creation.

You cannot have both for a language like JavaScript. So the question comes down to which is the right choice.

My reasoning to this problem is, at the end of the day, you still need to ensure your program is safe and sound. You can either make the language easy to write unsafe code, but hard to diagnose when developer made a mistake; or you can make the language encouraging developers to follow good practices, and provide useful diagnose support when they do make mistakes, while making it hard to write unsafe code.

JavaScript is often a language of convenience and has far more novice and hobbyist users than any other programming language. One of the goals of using is to take a large number of footguns related to resource management and distill it into a easily consumable syntax, but without sacrificing the flexibility and control that experienced developers need. There are often caveats and limitations that come along with that, and we must weigh those limitations against growing language complexity. I feel that using does a fair job at walking this line.

I also think that leak diagnosis will be less of a problem if runtimes provide easy-to-use safe handle wrappers like I mentioned above.

To show you what I meant by this, and you probably already realized, it's possible to convert the other approach into explicit imperitive style:

// THIS IS UNSAFE
function unsafeRetain(context) {
    let release;
    context.runAsync(new Promise(resolve => release = resolve));
    return release;
}

Now you can pass the release method freely - but unsafe. It's now functioning in the same way as Disposable - without the using syntax etc. It can leak too if you forget to call release(). But it doesn't disappear from heap dump. This is the main difference from bare Disposable - it still has the context awaiting for the dispose to happen so it preserves the retain path for diagnosis.

This does not function the same way as Disposable as it forces a dependency on asynchrony, and thus poisons synchronous code paths with await if they want to be able to properly handle exceptions thrown during disposal. It also doesn't make clear how those exceptions can be handled even in the async case, and potentially runs afoul of an unhandled promise rejection because the result of runAsync will go unwitnessed.

that is far too esoteric of a solution

It's harder to turn it into this form but for a good reason - it's unsafe to do so. But you can gain that level of freedom if you know what you were doing.

Therein lies the problem. An overly complex language feature will hamper adoption, and the benefits you've espoused with this design would do more to make the feature harder to reason over and harder to teach. using is simple. It may have a flaw with leaks, but I believe that flaw can be addressed with improvements to host APIs without sacrificing that simplicity. Those host APIs would need to be updated with your proposal as well.

4. Resource context capturing

I avoided using the dispose {} syntax so far because I wanted to seperate my context and intents. I want to dedicate this section for this topic.

The approach I proposed defined a contract to pass the resource context down the call stack. This has been done by explictly passing the context as function argument so far, but if we make it a language feature it may get a dedicated syntax.

As soon as you introduce scope capturing of an implicit surrounding scope, you have opened yourself up to memory and GC issues. If any callee can capture the implicit outer scope and save it, all of those resources lifetimes are no longer obvious to the caller.

As I mentioned above, although it's doable, it should be deemed unsafe and discoraged, unless you know what you are doing. This is at worst leaking with retain path info available. If you have nested DisposableStack binded together and you forgot to release the root stack, you leak everything on it too, without retain path info available.

And this capturing mechanism would need to keep alive all of the outer implicit scopes since there is no telling whether an inner resource has an ordered disposal dependency on an outer resource.

Yes. This is the same way if you constructed a nested DisposableStack, or have nested function calls each binded resources with using. This is the natural way nested resource dependencies should behave when the are released. You can break this pattern using unsafe methods of course, but we should discourage that.

No, this is not quite the same. A DisposableStack can be used to explicitly pass down lifetime, but otherwise has no effect on other using declarations. You opt-in to its use. What you've suggested so far is that syntax could be used to capture the surrounding scope. If you were able to capture the dispose scope within a nested function call, then this implies implicit pass-down of the scope and the scope's lifetime becomes unknowable. If you can only capture the dispose scope within the same lexical scope in which it was declared, then it is no different than just using const scope = new DisposableStack(), thus I don't see the benefit of syntax for that case.

Summary

I'd like to summarize my position as to what we've discussed so far:

Native implicit pass-down is not viable

Implicit pass-down of a dispose scope to permit capturing makes lifetime unknowable. If you cannot reason over the lifetime of a resource, then you cannot reliably use the feature. If you cannot reliably use the feature, then it is harder to teach and to learn and will hamper adoption. As a result, this is not a viable option for native support within the language.

If you "know what you're doing", you can easily implement implicit pass-down in your own code in such a way as to not affect surrounding code. You can do this by defining your own DisposableStack and passing it as an argument, or storing it in a field or variable or AsyncContext, and thus you know at the declaration site that the lifetime is potentially unknowable. This provides flexibility without sacrificing simplicity.

Hosts and built-ins can and should guard against leaky native handles

Yes, using can be leaky if you forget to use it. This can be guarded against for native resources by also leveraging FinalizationRegistry, and should be recommended in documentation, much as it is for C#. Host implementations like NodeJS and browsers should endeavour to guard resources in this way. This ensures that native resources are properly released when the handle itself is GC'd, and will also help with debugging and diagnostics when using heap snapshots.

Linters and type checkers can help as well

This can also be guarded against by linters and type checkers, even if only in part. For example, TypeScript might choose to implement a uses keyword to indicate to the caller that a Disposable will be tracked for cleanup, and provide an error if you fail to "use" it, return it, or store it in an outer variable or field:

class DisposableStack {
    ...
    use<T extends Disposable | null | undefined>(uses value: T): T & used;
    //                                           ^^^^                ^^^^
    //                                           |                   |
    // indicates the value passed to the parameter                   indicates the returned value should be considered
    //                should be considered as used                   as used
    ...
}

function foo() {
    const stack = new DisposableStack();
    //    ^^^^^ - ok: stack is returned, so its up to the caller to manage lifetime.

    const res1 = new SomeDisposable();
    //    ^^^^ - error: res1 was not used

    const res2 = new SomeDisposable();
    //    ^^^^ - ok: res1 is used below

    stack.use(res2);
    return stack;
}

function bar() {
    const res3 = new SomeDisposable() as used;
    //    ^^^^ - ok: explicitly declared as `used`, so we assume the user knows what they're doing
}

It should be easy to handle exceptions

It's important that a developer can reason over exceptions thrown during disposal. Both using and DisposableStack do this for you in a way that that works conveniently with try..catch and doesn't poison synchronous code with await just to retain lifetime.

Resource management should be easy to use

If we design this feature such that only the most experienced developers can use it reliably, then it won't be adopted by the majority. The inconvenience of APIs like FinalizationRegistry and Proxy are a necessity of their utility, and often only affect the producer of the object. The resulting code is often "write few, read many"—You only need to write a leak-safe Disposable around a native resource in a few places, yet the APIs that produce them will often be used orders-of-magnitude-more frequently, and by a combination of both experienced and inexperienced developers alike. It behooves us to design this feature with that in mind.

rixtox commented 1 year ago

1. Leak diagnose

I believe that runtimes will choose to adopt Disposable wrappers for most native handles, such as the NodeJS fs.promises.FileHandle, and they could easily leverage FinalizationRegistry for those wrappers. Runtimes may even choose to expose some kind of native handle wrapper as well:

interface SafeHandleData<T> {
    unsafeHandle: T;
    cleanup: (unsafeHandle: T) => void;
}

class SafeHandle<T> {
    static #dispose = ({ unsafeHandle, cleanup }: SafeHandleData<T>) => cleanup(unsafeHandle);
    static #registry = new FinalizationRegistry(SafeHandle.#dispose);
    #unregisterToken = {};
    #data: SafeHandleData<T> | undefined;

    constructor(unsafeHandle: T, cleanup: (unsafeHandle: T) => void) {
        this.#data = { unsafeHandle, cleanup };
        SafeHandle.#registry.register(this, this.#data, this.#unregisterToken);
    }

    get value() {
        if (!this.#data) throw new ReferenceError("Object is disposed");
        return this.#data.unsafeHandle;
    }

    dispose() {
        if (this.#data) {
            SafeHandle.#registry.unregister(this.#unregisterToken);
            const data = this.#data;
            this.#data = undefined;
            SafeHandle.#dispose(data);
        }
    }

    [Symbol.dispose]() {
        return this.dispose();
    }
}

It may even be worth a future add-on proposal to add something like SafeHandle natively.

If most native handles have Disposable wrappers, users won't need to reach for custom solutions, and thus those handles will either close on GC or appear in heap dumps. In other languages with this capability, the majority of user-written disposables are either some form of composition of existing disposables, or a disposable wrapper for some non-handle-based operation (such as tracing, transactions, etc.) that may not need such finalization behavior.

If all of the native resources you consume are already wrapped in some kind of safe handle wrapper, they will be readily visible in a heap snapshot.

I still think it's not an easy task to author safe Disposable resource even for engine developers. But that's easier for me, because I'm not the one authoring native resources. :joy:

Your response still doesn't address the undiagnosable nature of leaking by not using. We should not rely on linter to catch it. Linters can fail in may cases. I believe this can be done with a better language feature design.

Yes, leaks are possible with using. Yes, linters and type systems may not be able to be 100% reliable at catching this. However, I think this design is far more flexible than what you've suggested so far. Its far easier to write imperative code against Disposable when you explicitly don't want to leverage using. Its also far easier to integrate into the wealth of existing APIs without having to wholly restructure your code, which would otherwise be a severe adoption risk.

What you said here is not in alignment with what you previously said on another thread:

  1. We want the API to guide the user towards the safest and most reliable avenue for resource cleanup by making those use cases the easiest to address.
  2. We want to allow the user to do less safe things for specific cases, but there should be more resistance on that path to ensure the average use case stays on (1).

If you were being truthful to those doctrines, you would oppose to allowing Disposable to have leaky default behavior.

JavaScript is often a language of convenience and has far more novice and hobbyist users than any other programming language. One of the goals of using is to take a large number of footguns related to resource management and distill it into a easily consumable syntax, but without sacrificing the flexibility and control that experienced developers need. There are often caveats and limitations that come along with that, and we must weigh those limitations against growing language complexity. I feel that using does a fair job at walking this line.

Even in the name of usability I don't think you should relax number (1) that much. Memory/resource safety is hard - you either do it the hard way when you write it, or you learn it the hard way when you fix it. Your doctrine above was leaning toward the former, while your arguments to my concerns were emphasizing the later.

And more importantly, I believe what I proposed was not "sacrificing the flexibility and control that experienced developers need". I will outline a more complete contract in the next section to illustrate my point.

2. Sharing resource to multiple detached async calls

Intended resource lifetime can always vary based on need. using uses lexical block scoping to give you a clear boundary for a resource, while DisposableStack gives you both the ability to compose resources, as well as the ability to extend lifetime. If you want to have a resource lifetime that is associated with the execution context, you will already be able to do so programmatically with AsyncContext. But again, I am strongly opposed to natively supporting implicit pass down of disposable scopes as that approach is a GC hazard and can make lifetimes impossible to predict. That approach causes far more problems than it solves. If you wanted implicit pass down in your own code, you could do so with DisposableStack and it won't have any effect on using declarations in functions elsewhere in the call stack, so the authors of those functions can make reliable determinations about the lifetime of their resources.

You keep bringing up implicit pass down of disposable scopes even the examples I showed in this section have nothing to do with implicit scope capturing. I have a dedicated section below for that discussion because I don't want to pollute the context.

So let me rename the title of this section to clarify my intention, and address your related comments under the same context.

2. Resource context passing without implicit scope capturing

What I was trying to get to in this section was that resource context and execution context should be separated. using binds resource to block scope would quickly become not useful enough in many cases. Moving with DisposableStack can help, but it's adding unnecessary complexity making it harder to learn and write safe code. I completely agree with Ben on move being a overly complicated design. You can make it easier if you separated resource context and execution context from the begining.

Let me show you what I came up with as a simple programing contract to adopt resource context. I only did the async version but there could be a synchronous variant.

Let me emphasize that this is all without implicit scope capturing. What I want to get to in this section is that a programing contract can be established WITHOUT implicit scope capturing to achieve a safer resource management behavior than Disposable, without sacrifising usability, simple to learn, and library authors being more confident to adopt with a simple and safer contract having their back.

I call it the Lifecycle contract. The full implementation and type definitions can be found in this gist. Below is a detailed description of the contract. The text is quite long so I collapsed it to make it easier to navigate to my other comments. Expand it to have a read:

Description of the Lifecycle resource context contract

### Resource context and Lifecycle Resources are objects that require cleanup. The lifetime of a resource starts from it being created, and ends at it being cleaned up. ```js const { write, close } = open('file'); // created write('data'); close(); // cleaned up // cannot use write() again ``` A resource context owns resources by holding on to their teardown functions. A resource context can own many resources and perform cleanup all together. A resource has one and only one owning resource context and cannot be changed. A resource context has a Lifecycle of three stages: - Alive stage - when the resource context was accepting resource ownership. - Dying stage - when the resource context was starting to perform cleanup. - Dead stage - when the resource context was finished with all cleanup. A function can safely use a resource if the owning resource context was being kept alive throughout the function call. ```js const { write, close } = open('file'); // created life.defer(() => close()); // declare ownership to context, deferred teardown write('data'); // we can use it here because context is still alive (async () => { await sleep(1000); write('data'); // but how about here? how to ensure context is still alive? })(); ``` To run a function safely, we can extend the Lifecycles of all the depending resources' owning contexts to cover the lifetime of that function. To do this, we use `life.extend(function)`. ```js const { write, close } = open('file'); // created life.defer(() => close()); // declare ownership life.extend(async () => { // extend resource context Lifecycle await sleep(1000); write('data'); // safe to use }); ``` A function created a resource context should wait for the Lifecycle of that context to finish. Which is after all its resources were cleaned up. To create a new resource context, use `Lyfe.spawn(function)`. ```js import { Lyfe } from 'lyfe'; await Lyfe.spawn(async (life) => { const { write, close } = open('file'); life.defer(() => close()); write('data'); }); ``` Errors thrown from the function passed to `spawn()` and from the deferred callbacks during cleanup can be caught and handled by the await on `spawn()`. ### Declare resource context dependencies by spawning Lifecycle Some resources need to be kept alive for longer, while some resources are better to be cleaned up more frequently. Resource context only cleanup all resources it owns in once. Multiple resource contexts are needed to have different lifetimes. Resources have dependencies - a short living resource may depend on some long living resource. Therefore, resource contexts should also have dependencies - a short living context should keep the depending long living context alive. We achieve this by spawning short living context from a long living context. To do this, we use `life.spawn()`. ```js const { accept, close } = serve(); longLife.defer(() => close()); while (true) { await longLife.spawn(async (shortLife) => { const { write, close } = accept(); shortLife.defer(() => close()); shortLife.extend(async () => { await sleep(1000); write('data'); }); }); } ``` In practice, all Lifecycles are descendants from a root Lifecycle called `Lyfe`. ```js import { Lyfe } from 'lyfe'; Lyfe.spawn(async (life) => { // life is spawned from the root Lifecycle `Lyfe` }); ```

Cancellation with AbortController

> Note: Cancellation is not in-scope of our discussion, so I should just keep these collapsed. Because resources are often cleaned up when the task was cancelled, it's useful to provide resource management and cancellation in one simple-to-use API. [`AbortController`][AbortController] is used for [cancelling `fetch` requests](aborting_a_fetch). It's commonly available to many JavaScript environments and easy to import polyfill if not. Its simple API also made it a popular cancellation mechanism. To set an [`AbortSignal`][AbortSignal] when you spawn a Lifecycle, use `life.withAbort(abort)`. Then, the signal will be available on the child Lifecycle at `life.signal`. ```js const abort = new AbortController(); Lyfe.withAbort(abort).spawn(async (life) => { return fetch(url, { signal: life.signal }); }); ``` To register custom callback when an `abort` signal is received, use `life.onAbort(callback)`. ```js const { send, cancel, close } = makeRequest(); life.defer(() => close()); life.onAbort(() => cancel()); await send(); ``` By default, a child Lifecycle inherits the `AbortSignal` from the parent Lifecycle. A parent Lifecycle's `abort` signal will travel to all its descendant Lifecycles.
### The Lifecycle contract We define a simple contract for function and class authors to properly annotate their functions and classes, so they can be confident that when their code was used by other developers, the contract would enforce a set of safe behaviors to ensure resources to be properly cleaned up. In general, as a function or class author, you need to ask one simple question: is my function/class delegating resource cleanup tasks to the caller/creator? If the answer is no, you don't need to do anything to your function/class signature, just keep it like a normal function/class. However, if the answer is yes, you would need to adopt the Lifecycle contract to indicate to callers that some cleanup is delegated to them. On the other hand, if your function used resources that employed the Lifecycle contract, but you clean them up before your function returns, then your function doesn't delegate the cleanup to the caller, so you don't need to adopt the Lifecycle contract for this function. Rule of thumb: A function should try to contain the cleanup of its resources inside it. Only declare a Lifecycle parameter on the function signature if the function returns a resource that requires cleanup delegated to the caller. #### Value function If a function returns a value, not a resource that need to be cleaned up, write it as a simple function. ```ts function getValue(): Value; async function findValue(): Promise; ``` A function like this may use Resources inside it, but it performs cleanup before return, so the returned value doesn't require cleanup. ```ts async function findValue(): Promise { return Lyfe.spawn(async (life) => { const client = new Client(); life.defer(() => client.close()); return client.findValue(); }); } ``` You should always try to write a function in this way, and only adopt the following patterns when necessary.

Cancellable value function

> Note: Cancellation is not in-scope of our discussion, so I should just keep these collapsed. If a function returns a value that doesn't require cleanup, but accepts a cancellation signal, pass the signal in the parameters, instead of accepting a Lifecycle parameter: ```ts async function fetchValue(url: string, signal?: AbortSignal): Promise { const response = await fetch(url, { signal }); return response.json(); } async function findValue(signal?: AbortSignal): Promise { return Lyfe.withAbort(signal).spawn(async (life) => { const client = new Client(life); // client cancellable with the signal return client.findValue(); }); } ```
#### `LifeFunction` that returns resource If a function returns resource that requires cleanup delegated to the caller, it should accept the outer Lifecycle as its first parameter. A function satisfies this form is called a `LifeFunction`. ```ts async function findResource(outerLife: Lifecycle): Promise { return outerLife.spawn(async innerLife => { const client = new Client(innerLife); // client is cleaned up inside findResource const location = await client.locateResource(); const resource = new Resource(location); // resource is returned to caller outerLife.defer(() => resource.close()); // delegating cleanup to the outer Lifecycle return resource; }); } ``` Adopting the `LifeFunction` signature is requesting the caller to follow the contract to properly clean up the returned resource. Therefore, it requires the caller to pass in a Lifecycle to this function in someway. And if the caller returns the resource to even an outer caller, the required adoption of passing Lifecycle would bubble up the call stack until a function consumes the resource and performs cleanup before it returns. ### Interoperability with [`Disposable`][Disposable] To use `Disposable` or `AsyncDisposable` resource inside a Lifecycle context, just use `life.use(disposable)`. If one module adopts the Lifecycle contract, and another module consumes `AsyncDisposable`, you can wrap a Lifecycle into an `AsyncDisposable` using `life.unsafeSpawnAsyncDisposable()`. ```ts // You have a resource the follows the Lifecycle contract class Resource { constructor(life: Lifecycle) { const { read, write } = open(); life.defer(() => close()); this.read = read; } public read!: () => string; } // But some module consumes the resource expecting AsyncDisposable contract async function consumeAsyncDisposable(resource: AsyncDisposable & Resource) { using usedResource = resource; const data = resource.read(); } // You can convert Lifecycle into AsyncDisposable like this async function consumeAsyncDisposableWithLifecycle() { const resource = Lyfe.unsafeSpawnAsyncDisposable(life => new Resource(life)); await consumeAsyncDisposable(resource); } ``` ## (End of the Lifecycle contract)

The Lifecycle contract and the Disposable contract are similar in one way: they both alter a function's signature if there are cleanup delegated to the caller. But they achieve this in different styles:

The key difference between these contracts is not the position of the change to a function's signature, but rather the level of enforcement on the contract.

The Disposable contract only defines how a value should be produced, but it doesn't enforce how it should be consumed, thus having its "flexibility" while sacrifised safety and lack in confidence as a function author.

The Lifecycle contract on the other hand has "requiring" in its wording, so it's enforcing how its value should be consumed, but not by changing the return type.

// THIS IS UNSAFE
function unsafeRetain(context) {
    let release;
    context.runAsync(new Promise(resolve => release = resolve));
    return release;
}

Now you can pass the release method freely - but unsafe. It's now functioning in the same way as Disposable - without the using syntax etc. It can leak too if you forget to call release(). But it doesn't disappear from heap dump. This is the main difference from bare Disposable - it still has the context awaiting for the dispose to happen so it preserves the retain path for diagnosis.

This does not function the same way as Disposable as it forces a dependency on asynchrony, and thus poisons synchronous code paths with await if they want to be able to properly handle exceptions thrown during disposal.

I should have said AsyncDisposable instead. The Lifecycle contract I gave above has a utility method to turn a Resource using Lifecycle contract into an AsyncDisposable.

// You have a resource the follows the Lifecycle contract
class Resource {
    constructor(life: Lifecycle) {
        const { read, write } = open();
        life.defer(() => close());
        this.read = read;
    }
    public read!: () => string;
}

// But some module consumes the resource expecting AsyncDisposable contract
async function consumeAsyncDisposable(resource: AsyncDisposable & Resource) {
    using usedResource = resource;
    const data = resource.read();
}

// You can convert Lifecycle into AsyncDisposable like this
async function consumeAsyncDisposableWithLifecycle() {
    const resource = Lyfe.unsafeSpawnAsyncDisposable(life => new Resource(life));
    await consumeAsyncDisposable(resource);
}

The execution context calling the asyncDispose function should await for teardown completion. Currently I only implemented the async version but it's possible to have a specialized LifecycleSync type that only accepts synchronous defers, so it would be possible to be turned into a synchronous Disposable.

It also doesn't make clear how those exceptions can be handled even in the async case, and potentially runs afoul of an unhandled promise rejection because the result of runAsync will go unwitnessed.

It's important that a developer can reason over exceptions thrown during disposal. Both using and DisposableStack do this for you in a way that that works conveniently with try..catch and doesn't poison synchronous code with await just to retain lifetime.

My implementation can propagate teardown errors to the caller of asyncDispose. It should now behave like an AsyncDisposable and still be bound to a resource context.

And for Lifecycle contract this is also simple - whoever started a new Lifecycle context should await for resources to be cleaned up. Errors thrown from teardown functions can be caught too. Currently my implementation only report the first error seen, but it's easy to employ something like SuppressedError to collect all errors.

It's harder to turn it into this form but for a good reason - it's unsafe to do so. But you can gain that level of freedom if you know what you were doing.

Therein lies the problem. An overly complex language feature will hamper adoption, and the benefits you've espoused with this design would do more to make the feature harder to reason over and harder to teach. using is simple. It may have a flaw with leaks, but I believe that flaw can be addressed with improvements to host APIs without sacrificing that simplicity. Those host APIs would need to be updated with your proposal as well.

Let me emphasize that the Lifecycle contract I showed so far has no usage of implicit scope capturing. Lifecycle context was being passed explicitly so far. What I want to show here is how a better programing contract can be designed even without any change to the language syntax.

In terms of "complexity", I already showed above both the Disposable and Lifecycle contracts share the same amount of change toward a function adopting these contracts. And they can all be summarized into one or two easy-to-follow rules when authoring a function adopting these contracts. It's just the Lifecycle contract made it a requirement to change how the function is called, while Disposable is being optimistic in whether the caller would use the returned value correctly.

In addition, as discussed above, the move mechanism and dual-existence of block scoped and stack-like resource context are not "simple" at all. It's "simple" to just use Disposable, but much more complicated to use it correctly.

It would be great if you can show me some examples where the Lifecycle contract is more complicated than the Disposable contract.

4. Implicit resource context capturing

I have to say that I don't have a solid solution in implicit resource context capturing, but nor that I think this is that much a need for me. Even if we only have the Lifecycle contract as a 3rd party library would be enough for me. Having something similar in JavaScript standard library would be better even without implicit resource context capturing. It already solved all my biggest concerns with Disposable.

So my intent for doing implicit resource context capturing is solely for improving usability, and adding syntax level enforcement on the contract. I'm totally fine if it turned out to be not feasible and not having it at all. But the Disposable contract is just not meeting my expectation and should have something better, especially if it's meant for the JavaScript standard library and even syntax changes.

I understand it's already a stage 3 proposal, engines may have started implementation. I also wish I was told about it in an earlier stage. But I think this feature would become as important as async/await and deserves every chance to get improved before shipping.

What I was hoping to gain with a syntax level change is to enforce awaiting a new Lifecycle at where it's being created. This is a common pattern in using Lifecycle contract:

async function writeData(file, data) {
    await Lyfe.spawn(life => {
//  ^^^^^ - a new Lifecycle should always be awaited
        const { write } = open(life, file);
        write(data);
    });
}

Not awaiting here would make errors uncaught, violate resource lifetime dependencies, and generally a programming mistake. I haven't encountered a use case where it's okay to not await a new Lifecycle. So I wish there can be a syntax that: spawn a new Lifecycle, call a LifeFunction on the new Lifecycle context, and await for the new Lifecycle to finish.

As to whether should we make the Lifecycle context variable completely hidden and only accessible with special syntax is something I don't really have a preference over. This could be something like implicit resource context capturing but if there's a better way to do this I'm open to possibilities.

Native implicit pass-down is not viable

Implicit pass-down of a dispose scope to permit capturing makes lifetime unknowable.

I'm afraid I would need some examples to fully understand what you mean by having an unknowable lifetime. Are you suggesting that any function you called is able to indefinitely extend the lifetime of your resource context? I would argue if that's how long it takes for that function to use your resource, then you should be holding the resource context to that extent.

Also as mentioned in the Lifecycle contract discription above, if you have long living and short living resources, you can bind them to different Lifecycle contexts to have more fine-grained control over their lifetimes.

Or are you refering to one of the possibility I mentioned before that a function can capture and save a reference of a resource context to keep it alive indefinitely? I would agree that it's not a good idea at all. As I said I don't have a solid solution in this area yet.

If you "know what you're doing", you can easily implement implicit pass-down in your own code in such a way as to not affect surrounding code. You can do this by defining your own DisposableStack and passing it as an argument, or storing it in a field or variable or AsyncContext, and thus you know at the declaration site that the lifetime is potentially unknowable. This provides flexibility without sacrificing simplicity.

It's doable, and with AsyncContext it's probably doable even without using FinalizationRegistry. But I will avoid using it in production if it relies on GC to do leak free.

Hosts and built-ins can and should guard against leaky native handles

Yes, using can be leaky if you forget to use it. This can be guarded against for native resources by also leveraging FinalizationRegistry, and should be recommended in documentation, much as it is for C#. Host implementations like NodeJS and browsers should endeavour to guard resources in this way. This ensures that native resources are properly released when the handle itself is GC'd, and will also help with debugging and diagnostics when using heap snapshots.

Linters and type checkers can help as well

This can also be guarded against by linters and type checkers, even if only in part. For example, TypeScript might choose to implement a uses keyword to indicate to the caller that a Disposable will be tracked for cleanup, and provide an error if you fail to "use" it, return it, or store it in an outer variable or field:

class DisposableStack {
    ...
    use<T extends Disposable | null | undefined>(uses value: T): T & used;
    //                                           ^^^^                ^^^^
    //                                           |                   |
    // indicates the value passed to the parameter                   indicates the returned value should be considered
    //                should be considered as used                   as used
    ...
}

function foo() {
    const stack = new DisposableStack();
    //    ^^^^^ - ok: stack is returned, so its up to the caller to manage lifetime.

    const res1 = new SomeDisposable();
    //    ^^^^ - error: res1 was not used

    const res2 = new SomeDisposable();
    //    ^^^^ - ok: res1 is used below

    stack.use(res2);
    return stack;
}

function bar() {
    const res3 = new SomeDisposable() as used;
    //    ^^^^ - ok: explicitly declared as `used`, so we assume the user knows what they're doing
}

You don't need any of these for the Lifecycle contract. It's already being enforced in function signature.

It should be easy to handle exceptions

It's important that a developer can reason over exceptions thrown during disposal. Both using and DisposableStack do this for you in a way that that works conveniently with try..catch and doesn't poison synchronous code with await just to retain lifetime.

You can also use try..catch together with Lifecycle contract. await is only required at the call site that created an async Lifecycle - where it accepts async defer callbacks, to collect the completion of the Lifecycle. As I mentioned, it's possible to write a synchronous version that only accepts synchronous defer callbacks, so you don't need to await. This is the same as a function that using an AsyncDisposable would be "polluted" to be async.

Resource management should be easy to use

If we design this feature such that only the most experienced developers can use it reliably, then it won't be adopted by the majority.

As I argued aboved, while it has a seemingly simple syntax, it's difficult to use Disposable contract "reliably".

The inconvenience of APIs like FinalizationRegistry and Proxy are a necessity of their utility, and often only affect the producer of the object. The resulting code is often "write few, read many"—You only need to write a leak-safe Disposable around a native resource in a few places, yet the APIs that produce them will often be used orders-of-magnitude-more frequently, and by a combination of both experienced and inexperienced developers alike. It behooves us to design this feature with that in mind.

I understand where you are getting with this. But it's still something native resource authors have to opt-in for. Developers will start wrapping ANYTHING inside a Disposable starting on day 1 - node:fs, node:net, you name it. And I'm sure not many of those Disposables can be "leak-safe". We will end up with a landscape of a few "safe" and many "unsafe" Disposables all over the place.

rbuckton commented 1 year ago

1. Leak diagnose

I still think it's not an easy task to author safe Disposable resource even for engine developers. But that's easier for me, because I'm not the one authoring native resources. 😂

If the perscriptive guidance for a Disposable over a native handle is to also use FinalizationRegistry, I think implementers or engine hosts (such as browsers) wouldn't have difficulty. They could either leverage FinalizationRegistry directly via spec-native intrinsics, or we could look into adding a host API to make that easier.

Yes, leaks are possible with using. Yes, linters and type systems may not be able to be 100% reliable at catching this. However, I think this design is far more flexible than what you've suggested so far. Its far easier to write imperative code against Disposable when you explicitly don't want to leverage using. Its also far easier to integrate into the wealth of existing APIs without having to wholly restructure your code, which would otherwise be a severe adoption risk.

What you said here is not in alignment with what you previously said on another thread:

  1. We want the API to guide the user towards the safest and most reliable avenue for resource cleanup by making those use cases the easiest to address.
  2. We want to allow the user to do less safe things for specific cases, but there should be more resistance on that path to ensure the average use case stays on (1).

I also said in this thread that the decisions we make must be balanced with ease of use and teachability. This was also about the API, and I've already discussed how leaking native handles and heap snapshot discoverability are already solvable via an API, where hosts offer convenient and safe handle wrappers for native APIs that do this work for you.

If you were being truthful to those doctrines, you would oppose to allowing Disposable to have leaky default behavior.

These are not doctrines, they are guiding principles, and I'm willing to bend them where necessary to make something that is useable. This proposal is easy to stitch onto existing NodeJS and DOM APIs without requiring a significant rewrite, or requring that users use wholly different methods to use, which your suggestions would likely entail.

2. Resource context passing without implicit scope capturing

What I was trying to get to in this section was that resource context and execution context should be separated. using binds resource to block scope would quickly become not useful enough in many cases. Moving with DisposableStack can help, but it's adding unnecessary complexity making it harder to learn and write safe code. I completely agree with Ben on move being a overly complicated design. You can make it easier if you separated resource context and execution context from the begining.

There is far too much in this section to discuss point by point, so I will summarize my position.

I think we have very different positions on how a resource's lifetime should be scoped. Lifecycle, as you have suggested, is very similar to DisposableStack. You can, if you so chose, pass down a DisposableStack as a parameter to register resources in the same way. Where they differ is in who controls lifetime. With a Lifecycle, the lifetime of the object is completely out of control of the resource consumer (in this case, the author of the Lifecycle instance). While it seems as if the Lifecycle creator owns the lifetime, they in fact give that control over to any function they pass it to as a parameter, as any of those functions could chose to extend() the lifetime beyond that which the Lifecycle creator expects. This design can lead to race conditions, which can be far harder to diagnose than leaks.

On the other hand, DisposableScope ensures the lifetime is bounded in a predictable way, as its lifetime is effectively owned by the creator. That may seem like a weak position, given that any callee could effectively call .move() to change the lifetime as well, except that a DisposableScope is an example of an affine type in a substructural type system. An affine type is one that can be used at most once, and that behavior is represented by .move():

const a = new DisposableStack();
a.use(getResource());

const b = a.move(); // 'b' is now the owner of the resource
const c = b.move(); // 'c' is now the owner of the resource

a.move(); // throws as resources have already been moved out of 'a'.

As a result of this behavior, the lifetime of a DisposableScope can be reliably known. If you are the creator, you can only .move() resources out of it if no one else has. If .move() doesn't throw, the creator knows they still own the lifetime. If it does throw, then the creator no longer knows the lifetime, and an exception is thrown. By the way, Rust's move semantics are another example of affine types, and are the inspiration behind the method name move() as well.

In your response you indicated that you believe "resource context and execution context should be separated", but I disagree. A resource's lifetime should be explicit, bounded, and knowable so as to avoid race conditions and properly order resource dependencies and dependent resource cleanup. If that lifetime does not have a known, explicit boundary, then you cannot make any reliable assumptions about resource state. This introduces data races and deadlocks, which cannot be solved as readily as leaks can. using declarations and DisposableStack both provide explicit lifetime boundaries and ownership guarantees:

If you want your resource lifetime to be divorced from execution context, we already have FinalizationRegistry. With a Lifecycle, extending a lifetime requires a method call, in an appropriate async context, and may still be even tied to the Lifecycle instance until it is GC'd. With a FinalizationRegistry you can "extend" lifetime by merely holding the resource, which is a far more obvious implication.

As I see it, you have essentially suggested three things:

  1. A way to ensure native handles aren't leaked (a) by mandating that an enclosing resource scope is provided at the time the resource is allocated (b).
  2. A way to make obvious an open native handle in a heap snapshot.
  3. A way to extend the lifetime of a resource beyond its initial lifetime.

I believe that (1.a) and (2) can be handled by hosts implementing appropriate safe native handle wrappers that leverage [Symbol.dispose]() and FinalizationRegistry. Most users will not need this level of complexity.

I believe that (1.b) is an unnecessary guard rail if safe native handles are provided by hosts. It also overcomplicates resource composition scenarios, and potentially requires huge API changes for the entire ecosystem to adopt.

I believe that (3) can be accomplished with DisposableStack, and in a way that guarantees the correct ownership of resource lifetime. I believe the approach offered by Lifecycle to be a very dangerous footgun as it makes lifetime extension non-obvious to the lifetime creator.

I believe that using and DisposableStack do a far better job at avoiding race conditions than Lifecycle does.

Finally, I don't see how using Lifecycle/Lyfe could work in non-async code. The only way to extend lifetime I can see would be to use an async function and resolve() it later (as you suggested), which makes it impossible to catch disposal exceptions in synchronous code. This complexity makes resource composition very difficult to do correctly.

rixtox commented 1 year ago

I think we have very different positions on how a resource's lifetime should be scoped. Lifecycle, as you have suggested, is very similar to DisposableStack. You can, if you so chose, pass down a DisposableStack as a parameter to register resources in the same way. Where they differ is in who controls lifetime. With a Lifecycle, the lifetime of the object is completely out of control of the resource consumer (in this case, the author of the Lifecycle instance). While it seems as if the Lifecycle creator owns the lifetime, they in fact give that control over to any function they pass it to as a parameter, as any of those functions could chose to extend() the lifetime beyond that which the Lifecycle creator expects. This design can lead to race conditions, which can be far harder to diagnose than leaks.

I see what you mean, and it's a valid point. I only considered a function declaring a Lifecycle as input parameter for delegating cleanup to the caller, but didn't consider enough what it means by passing a Lifecycle to a function that is consuming a resource bound to that Lifecycle on a detached async context. You are right that it makes it harder to reason about the implication to resource lifetime when being used in that particular case. I don't have a solution to that question yet. So I will withdraw from pushing a change toward passing resource context.

If you want your resource lifetime to be divorced from execution context, we already have FinalizationRegistry. With a Lifecycle, extending a lifetime requires a method call, in an appropriate async context, and may still be even tied to the Lifecycle instance until it is GC'd. With a FinalizationRegistry you can "extend" lifetime by merely holding the resource, which is a far more obvious implication.

I just learned that Node.js FileHandle is already being made to be auto closed on GC but not based on FinalizationRegistry. So they already have the mechanism to do some of those things you suggested.

It would probably be better if there's a standard utility to help non-engine developers to author "garbage-collectable" Disposables. It can reduce the complexity in writing custom FinalizationRegistry integrations if developers want to wrap legacy handles in Disposable and opt-in for GC cleanup. Maybe something like FinalizationRegistry.disposeOnGC(disposable): void? But the question would be how can it know if a resource has been disposed when doing GC? There's no disposable.disposed: boolean field in the Disposable interface.

Probably something like FinalizationRegistry.makeDisposable<T extends object>(object: T, dispose: () => void): Disposable & T and FinalizationRegistry.makeAsyncDisposable<T extends object>(object: T, dispose: () => Promise<void>): AsyncDisposable & T can work better? But it would then raise question on how to use it for classes.

rbuckton commented 1 year ago

It would probably be better if there's a standard utility to help non-engine developers to author "garbage-collectable" Disposables. It can reduce the complexity in writing custom FinalizationRegistry integrations if developers want to wrap legacy handles in Disposable and opt-in for GC cleanup. Maybe something like FinalizationRegistry.disposeOnGC(disposable): void? But the question would be how can it know if a resource has been disposed when doing GC? There's no disposable.disposed: boolean field in the Disposable interface.

This is most likely not an option. When FinalizationRegistry was designed, it was an intended goal to avoid the complexity of resurrection and reachability semantics of GC in other languages, like that of C#. As such, a FinalizationRegistry cannot itself hold a reference to the garbage collected object. This is why the register() method takes three arguments: The target object whose collection will be monitored, a "held value" that can be used to perform actual cleanup when passed to the callback supplied to the registry's constructor, and an optional "unregister token" that can be used to remove the registry entry.

A disposeOnGC method would need to both monitor disposable and use it as the this when performing cleanup. Since neither FinalizationRegistry nor the JavaScript GC has the concept of object resurrection, this would mean that the disposable would be held indefinitely and could never be GC'd.

Probably something like FinalizationRegistry.makeDisposable<T extends object>(object: T, dispose: () => void): Disposable & T and FinalizationRegistry.makeAsyncDisposable<T extends object>(object: T, dispose: () => Promise<void>): AsyncDisposable & T can work better? But it would then raise question on how to use it for classes.

This is closer, but still has issues that prevent it from being a viable approach. It would again need to differentiate between the object whose GC is monitored (i.e., object) and some value that can be used to identify the resource for cleanup (i.e., the "held value"). Accepting a dispose callback that takes no arguments is likely to result in the user introducing a closure that captures object, which would, in turn, result in a cycle that prevents GC. It also seems to work by mutating an existing value, which isn't a common practice in the standard library. Finally, in most cases where you would need this functionality, you are often working with a native handle or file descriptor, which is usually represented by a number, not an object.

This is where the SafeHandle class I suggested earlier would make more sense. However, that suggestion was predicated on it being a host-defined capability rather than something that is part of the standard library. The main reason I would not propose SafeHandle for ECMA262 is that the core language itself has no notion of a "handle", nor does it give any particular meaning to a number. Instead, this meaning is given by a host like NodeJS and its file descriptors, or the DOM and its timers.

In any case, I'm not sure its wholly necessary to design an API for such use cases as native handle wrappers. These can more readily be handled by hosts themselves, who often have no need for a user-facing API as you are proposing. While I don't see it as being necessary for the MVP for this feature, it's definitely something we could pursue as a follow-on proposal given adequate feedback.

rixtox commented 1 year ago

It would probably be better if there's a standard utility to help non-engine developers to author "garbage-collectable" Disposables. It can reduce the complexity in writing custom FinalizationRegistry integrations if developers want to wrap legacy handles in Disposable and opt-in for GC cleanup. Maybe something like FinalizationRegistry.disposeOnGC(disposable): void? But the question would be how can it know if a resource has been disposed when doing GC? There's no disposable.disposed: boolean field in the Disposable interface.

This is most likely not an option. When FinalizationRegistry was designed, it was an intended goal to avoid the complexity of resurrection and reachability semantics of GC in other languages, like that of C#. As such, a FinalizationRegistry cannot itself hold a reference to the garbage collected object. This is why the register() method takes three arguments: The target object whose collection will be monitored, a "held value" that can be used to perform actual cleanup when passed to the callback supplied to the registry's constructor, and an optional "unregister token" that can be used to remove the registry entry.

I'm sure many people found that register() semantics confusing, but I understand why it was designed that way. Which is why I would like to see a simple interface just for GC-guarding Disposable instead of letting people to go through the complexity of learning now garbage collector works.

A disposeOnGC method would need to both monitor disposable and use it as the this when performing cleanup. Since neither FinalizationRegistry nor the JavaScript GC has the concept of object resurrection, this would mean that the disposable would be held indefinitely and could never be GC'd.

Probably something like FinalizationRegistry.makeDisposable<T extends object>(object: T, dispose: () => void): Disposable & T and FinalizationRegistry.makeAsyncDisposable<T extends object>(object: T, dispose: () => Promise<void>): AsyncDisposable & T can work better? But it would then raise question on how to use it for classes.

This is closer, but still has issues that prevent it from being a viable approach. It would again need to differentiate between the object whose GC is monitored (i.e., object) and some value that can be used to identify the resource for cleanup (i.e., the "held value"). Accepting a dispose callback that takes no arguments is likely to result in the user introducing a closure that captures object, which would, in turn, result in a cycle that prevents GC. It also seems to work by mutating an existing value, which isn't a common practice in the standard library. Finally, in most cases where you would need this functionality, you are often working with a native handle or file descriptor, which is usually represented by a number, not an object.

Instead of mutating the object passed to makeDisposable, would it work better if it returns a Proxy object? It would also allow capturing the object in the dispose closure.

This is where the SafeHandle class I suggested earlier would make more sense. However, that suggestion was predicated on it being a host-defined capability rather than something that is part of the standard library. The main reason I would not propose SafeHandle for ECMA262 is that the core language itself has no notion of a "handle", nor does it give any particular meaning to a number. Instead, this meaning is given by a host like NodeJS and its file descriptors, or the DOM and its timers.

In any case, I'm not sure its wholly necessary to design an API for such use cases as native handle wrappers. These can more readily be handled by hosts themselves, who often have no need for a user-facing API as you are proposing. While I don't see it as being necessary for the MVP for this feature, it's definitely something we could pursue as a follow-on proposal given adequate feedback.

If we don't have a solution to this problem in this proposal, I would suggest making it clear in the API guideline that wrapping native handles inside Disposables can potentially leak and should be advised against.

rbuckton commented 1 year ago

I'm sure many people found that register() semantics confusing, but I understand why it was designed that way. Which is why I would like to see a simple interface just for GC-guarding Disposable instead of letting people to go through the complexity of learning now garbage collector works.

There is no "simple interface for just GC-guarding" a disposable without completely rearchitecting GC in JavaScript. You cannot wait for GC on an object while still using that object to perform its own cleanup, as that means the object will always have some kind of reference after it has been GC'd, so it can never be GC'd. FinalizationRegistry was the simplest solution to the problem without needing to completely rearchitect GC on every engine.

Instead of mutating the object passed to makeDisposable, would it work better if it returns a Proxy object? It would also allow capturing the object in the dispose closure.

You cannot create a proxy for a primitive value, and a proxy still holds its target strongly, so a Proxy does not solve any of the concerns I presented.

SafeHandle is pretty much the simplest mechanism. You could potentially subclass it, but you would still need to pass a "held value" and a cleanup callback to the super constructor. Overriding [Symbol.dispose] would have no effect on finalization, however.

// node:fs
function closeSync(fd) { ... }

class FileHandle extends SafeHandle {
  constructor(fd) {
    super(fd, closeSync); // finalizer does not strongly hold a `FileHandle`, receives `fd` as input
  }
}

If we don't have a solution to this problem in this proposal, I would suggest making it clear in the API guideline that wrapping native handles inside Disposables can potentially leak and should be advised against.

That is my intent, yes. One actionable item might be to add a non-normative NOTE to the specification that includes this information as an advisory for hosts.

bathos commented 1 year ago

Aside: This thread has been instructive — the responses have helped me understand the API and the motivations behind its design decisions better than I’d previously been able to work out from the readme/explainer.

rixtox commented 1 year ago

Where they differ is in who controls lifetime. With a Lifecycle, the lifetime of the object is completely out of control of the resource consumer (in this case, the author of the Lifecycle instance). While it seems as if the Lifecycle creator owns the lifetime, they in fact give that control over to any function they pass it to as a parameter, as any of those functions could chose to extend() the lifetime beyond that which the Lifecycle creator expects. This design can lead to race conditions, which can be far harder to diagnose than leaks.

I have been thinking more on this issue lately. I agree with you the Lifecycle contract I introduced can in some cases making it harder to understand the boundary of resource lifetime if you passed the Lifecycle object to a function - the function MAY intend to extend the lifetime of that Lifecycle, and hence all the resources registered to that Lifecycle scope. There's no explicit expression of that intention on a function's signature - making it ambiguous between a function that defers resource cleanup to the caller, and a function that consumes resource while extending its lifetime asynchronously passes its function return.

That being said, if we put the Lifecycle contract thing aside (which has some design issues), it's still a valid practice to let a callee function extend some lifetime passed in from the caller.

First thing that comes to mind is WaitGroup in Golang. It's quite a common practice in Go to spawn multiple Go routines, wait till all Go routines to finish, and resume the original thread. Illustrated by the following example from gobyexample.com:

package main

import (
    "fmt"
    "sync"
    "time"
)

func worker(id int) {
    fmt.Printf("Worker %d starting\n", id)
    time.Sleep(time.Second)
    fmt.Printf("Worker %d done\n", id)
}

func main() {
    var wg sync.WaitGroup
    for i := 1; i <= 5; i++ {
        wg.Add(1)
        i := i
        go func() {
            defer wg.Done()
            worker(i)
        }()
    }
    wg.Wait()
}

In this pattern, the caller is "giving control" of the WaitGroup's lifetime to the Go routines that captured it, (or passed in as parameter). It's explicit with the intention here - before it spawned a Go routine, the caller would do wg.Add(1) indicating it EXPECTS the Go routine to extend the lifetime, and call wg.Done() at some point. It's similar to doing manual ref-counting for the WaitGroup lifetime.

In fact any ref-count like contracts would work in a similar way - you extend the lifetime by calling retain(), and the last one to call release() (when ref-count drops to 0) would initiate the disposal. No single function or execution context really "owns" that lifetime, or in a very abstract way, we can say the ref-counter "owns" the lifetime. Does it make it harder to reason about the exact point where the dispose would be initiated? Yes. But does it really matter? Probably no, because the contract would work correctly if you just retain before you use, and release after you done, then you would not need to care where is the exact point of disposal.

Moreover, WaitGroup.Wait() provided a way to know when is safe to continue execution after the WaitGroup lifetime ends. So instead of controlling when to dispose a lifetime, it provided a way to observe when a lifetime is disposed. This is actually an easier contract to follow. Because reasoning an exact point to explicitly dispose something, across many different functions that use that resource is harder and more error-prone than simply reason about how a function uses some resource locally, and leverage reference counting to do the book-keeping for you.

Similar practices are found in classical synchronization patterns like Semaphore, or even just by spawning and joining threads.

Resources should get cleaned up at a predictable point, not based on what other stuff happens to be on the callstack.

So I disagree with this point you made. If a contract made it explicit that a callee function intends to extend lifetime, and there's a mechanism to wait or join lifetime, then it wouldn't be a problem if you cannot predict the exact point of disposal.

We can bring the same mechanism to JavaScript. We can create inline closure to explicitly extend lifetime on some detached async context (Go routine/Promise), and later join/await them in the same calling context. This would make extending lifetime more explicit and easy to follow. We can already do this with Promise.allSettled(). Let's pretend we don't have extend() on the Lifecycle interface:

async function main() {
    await Lyfe.spawn(async (life) => {
        const names = ['file1', 'file2', 'file3'];
        const promises = names.map(async (name) => { // concurrent promises
            // inline closure to spawn concurrent detached promises
            const handle = createHandle(life, name);
            await handle.write('data');
        });
        const results = await Promise.allSettled(promises); // joining promises
        handleResultsAndErrors(results); // error handling
    });
}

So in some degree, Promise.allSettled() is already behaving similar to a WaitGroup. It's just a bit inconvenient to handle results and errors from the Promise group, which can be improved with some utility functions.

Some side notes on cancellation. It's generally a good practice to support cancellation if you are authoring a function that could extend a lifetime indefinitely - when a remote endpoint is being unresponsive, or some operation is just taking too long. Cancellation would grant the caller more control on the execution lifetime. The Lifecycle contract integrated AbortController and AbortSignal mechanisms to do that.

Jamesernator commented 1 year ago

So the root problem feels like it could more simply resolved by instead an __enter__/__exit__ style thing like Python has. i.e. For objects that really should be cleaned up everytime, you should have an intermediate object that vends the objects that should be cleaned up.

Like we can do this in Python:

# Vended only by SafeOpen
class FileHandle:
    def read(self, count):
        # read some bytes
        pass

    def close(self):
        # whatever to close
        pass

class SafeOpen:
    def __init__(self, file_name):
        self._file_handle = FileHandle(...)

    def __enter__(self):
        return self._file_handle

    def __exit__(self):
        self._file_handle.close()

handle = SafeOpen("somefile.txt")
# This doesn't work because handle isn't a FileHandle
handle.read()

# But this does
with SafeOpen("somefile.txt") as handle:
   handle.read()

For JS we could just do a similar thing and have a [Symbol.create] in addition to [Symbol.dispose] which is called to create the object in the using statement.

i.e. We could make using equivalent to:

using handle = objProvider;
BODY();
// Get the function that creates the handle
const $$makeDisposable = objProvider[Symbol.create] ?? (() => objProvider);
const handle = $$makeDisposable();
const $$disposer = handle[Symbol.dispose];
try {
    BODY();
} finally {
    $$disposer.call(handle);
}

With this we could have the advantages of both the current design, i.e. disposing existing objects, and the benefits of Python's design, i.e. being able to control creation.

(And sure people in userland could still forget to cleanup the resource if they do:

const handle = objProvider[Symbol.create]();

but why would they bother calling [Symbol.create]() directly when using is a thing?)


In such a scheme the original example could simply be changed to:

function createHandle() {
    return {
        [Symbol.create]() {
            const { write, close } = nativeHandle();
            return {
                write,
                [Symbol.dispose]() {
                    close();
                },
            };
        },
    };
}

// Hey this now does nothing but return the { [Symbol.create]() { ... } } object
const handle = createHandle();
// TypeError: write is not a function
handle.write();

// But this works
using handle = createHandle();
handle.write();
rixtox commented 1 year ago
function createHandle() {
    return {
        [Symbol.create]() {
            const { write, close } = nativeHandle();
            return {
                write,
                [Symbol.dispose]() {
                    close();
                },
            };
        },
    };
}

// Hey this now does nothing but return the { [Symbol.create]() { ... } } object
const handle = createHandle();
// TypeError: write is not a function
handle.write();

// But this works
using handle = createHandle();
handle.write();

I like it. And I think it's not far from what I proposed. To show what I mean, we can have helpers to make it easier to write your new "Symbol.create" object. So something like:

function DisposableHelper(createFunction) {
    function create() {
        const teardowns = [];
        function defer(teardown) {
            teardowns.push(teardown);
        }
        function dispose() {
            let error;
            let hasError = false;
            while(teardowns.length) {
                try {
                    teardowns.pop()();
                } catch (err) {
                    error = new SuppressedError(error, err);
                    hasError = true;
                }
            }
            if (hasError) {
                throw error;
            }
        }
        const obj = createFunction(defer);
        return Object.assign(obj, { [Symbol.dispose]: dispose });
    }
    return { [Symbol.create]: create };
}

And your createHandle() function can be simplified to this:

function createHandle(defer) {
    const { write, close } = nativeHandle();
    defer(() => close());
    return { write };
}

using handle = DisposableHelper(createHandle);
handle.write();

This helper is actually very close to what I proposed. And in fact if you make the helper a language level feature, maybe directly what using can transpile into, then you can write things like this:

using handle = createHandle();

and get transpiled into this:

const handle = createHandle(localStack.defer);

And in this form it's the same thing I proposed earlier:

const handle = createHandle(lifecycle.defer);

If we do it this way, we don't even need Symbol.dispose or Symbol.create. What we really need are these things:

  1. A helper/syntax to create a dispose scope / stack with a defer method. We already have DisposableStack but not a syntax for this.
  2. Feed the defer method into a resource creation function. This can be simply function argument.
  3. Make sure the dispose scope / stack is bound to some execution context and get disposed properly.

One way to do this is to have the using keyword to be transplied like this:

using handle = createHandle();
// transpiled directly into this
const handle = createHandle(defer);

So whatever defer refers to at this line get passed to the resource creation function.

In fact it would become too trivial of a syntatic sugar and probably just fine if you remove the using syntax, and just pass defer as an argument instead.

And then you have another syntax to quickly create the defer function with proper disposal binding:

dispose {
    // defer() is available in this block
    const { write, close } = nativeHandle();
    defer(() => close());
    using handle = createHandle();
}
// everything added to defer() got disposed when leaving this block

// and for async version

async dispose {
    // defer() accepts async functions
    const { writeAsync, closeAsync } = nativeAsyncHandle();
    defer(() => closeAsync());
    // without the `using` keyword can still be simple:
    const handle = createAsyncHandle(defer);
}
// everything added to defer() got disposed and awaited

Of course you can still manually create defer using helper, and it's what the above transpile into:

DisposeScope((defer) => {
    const { write, close } = nativeHandle();
    defer(() => close());
    const handle = createHandle(defer);
});

// and for async version

await AsyncDisposeScope(async (defer) => {
    // defer() accepts async functions
    const { writeAsync, closeAsync } = nativeAsyncHandle();
    defer(() => closeAsync());
    const handle = createAsyncHandle(defer);
});

These helpers resemble Promise creation signature in some degree.

Any function that want to delegate cleanup to caller can just declare defer as function parameter:

function prepareHandle(defer, handshakeData) {
    using handle = createAsyncHandle();
    await handle.write(handshakeData);
    return handle;
}
// transpiled into
function prepareHandle(defer, handshakeData) {
    const handle = createAsyncHandle(defer);
    await handle.write(handshakeData);
    return handle;
}

Of course, taking this form would get rid of the Disposable type, and probably also the using keyword, making the language less like C#. While introduing defer but not necessarily a keyword, probably making it a bit resembling Golang.

zaygraveyard commented 1 year ago

@rixtox No offense to you or what you're proposing 😅, but as I understood from the comments of @rbuckton in this thread, adding a function parameter is a nonstarter because it would be an adoption risk:

Its also far easier to integrate into the wealth of existing APIs without having to wholly restructure your code, which would otherwise be a severe adoption risk.

https://github.com/tc39/proposal-explicit-resource-management/issues/159#issuecomment-1584900038

I really like @Jamesernator's proposal and hope it can still be considered, since it supports your use case as well (albeit with a helper function).

rixtox commented 1 year ago

@rixtox No offense to you or what you're proposing 😅, but as I understood from the comments of @rbuckton in this thread, adding a function parameter is a nonstarter because it would be an adoption risk:

Its also far easier to integrate into the wealth of existing APIs without having to wholly restructure your code, which would otherwise be a severe adoption risk.

#159 (comment)

I really like @Jamesernator's proposal and hope it can still be considered, since it supports your use case as well (albeit with a helper function).

You need to be more specific about "adoption risk". Ron's comment of "wholly restructure your code" needs more clarification. I can do a side-by-side comparison of adoption efforts required for my proposal and Ron's Disposable and show you it's really about the same level of adoption efforts if not easier, if you want to correctly cleanup resources.

Case 1: Inline resource management

In this case, you create a native resource handle, use it, and dispose it all in the same function body.

For vallina JS, you can write it using try block but needs to be careful about error catching:

function main() {
    const { read, closeReader } = nativeReader();
    try {
        const data = read();
        const { write, closeWriter } = nativeWriter();
        try {
            write(data);
        } finally {
            closeWriter();
        }
    } finally {
        closeReader();
    }
}

Using Ron's Disposable design, you can use the DisposableStack in this case to not wrap handles in Disposable and just use them inline:

function main() {
    const stack = new DisposableStack();
    try {
        const { read, closeReader } = nativeReader();
        stack.defer(() => closeReader());
        const data = read();
        const { write, closeWriter } = nativeWriter();
        stack.defer(() => closeWriter());
        write(data);
    } finally {
        stack.dispose();
    }
}

And you can get rid of the try block by adopting the using syntax:

function main() {
    using stack = new DisposableStack();
    const { read, closeReader } = nativeReader();
    stack.defer(() => closeReader());
    const data = read();
    const { write, closeWriter } = nativeWriter();
    stack.defer(() => closeWriter());
    write(data);
}

With what I proposed, you can manually create a DisposeScope:

function main() {
    DisposeScope((defer) => {
        const { read, closeReader } = nativeReader();
        defer(() => closeReader());
        const data = read();
        const { write, closeWriter } = nativeWriter();
        defer(() => closeWriter());
        write(data);
    });
}

Which is already a slightly easier to use than DisposableStack + try block. And if we have a syntax to create the DisposeScope it can be written as:

function main() {
    dispose {
        const { read, closeReader } = nativeReader();
        defer(() => closeReader());
        const data = read();
        const { write, closeWriter } = nativeWriter();
        defer(() => closeWriter());
        write(data);
    }
}

Case 2: Function that returns resource that requires cleanup by caller

This use case is most commonly used for wrapping native handles in a creation function, and is the main use case both Ron's and my proposals are trying to address.

For traditional vallina JS, this is just the same as native resource creation function:

type NativeHandle = {
    write(data: string): void;
    close(): void;
};
declare function createHandle(): NativeHandle;

// to use it you need to use try block as shown earlier

const { write, close } = createHandle();
try {
    write(data);
} finally {
    close();
}

To adopt Disposable contract, you need to change the NativeHandle return type into a DisposableHandle type:

type DisposableHandle = {
    write(data: string): void;
    [Symbol.dispose](): void;
};
declare function createHandle(): DisposableHandle;

const { write, [Symbol.dispose]: dispose } = createHandle();
try {
    write(data);
} finally {
    dispose();
}

And to ease the usage to get rid of try block, you can adopt using syntax:

type DisposableHandle = {
    write(data: string): void;
    [Symbol.dispose](): void;
};
declare function createHandle(): DisposableHandle;

{
    using { write } = createHandle();
    write(data);
}

To adopt my proposal, you need to change the createHandle() function to accept a defer argument, and you can (optionally) get rid of the close() method on the returned type:

type Handle = {
    write(data: string): void;
};
declare function createHandle(defer: Deferrer): Handle;

DisposeScope((defer) => {
    const { write } = createHandle(defer);
    write(data);
});

And you can adopt the syntax to create the scope easier:

type Handle = {
    write(data: string): void;
};
declare function createHandle(defer: Deferrer): Handle;

dispose {
    const { write } = createHandle(defer);
    write(data);
}

I hope this can show you the level of adoption efforts of the two approaches are basically the same. The differences lie in the mind set when authroing and using these contracts.

Ron's proposal changes the return type of a function. As a function author, you have no way to tell if your return value can be properly used by a caller. My proposal changes the parameter to a function. As a function author, you can be sure a caller has to provide the correct arguments to use this function. This key difference changes the level of safeness when authoring and using our contracts.

So if you need the same level of adoption effort to correctly use the two contract, and one contract can enforce a better discipline to prevent misuse, I would prefer the safer one.

Backward compatibility

It's true the Disposable contract can maintain backward compatibility when upgrading a native resource creation function to return a Disposable type without changing the function's name or parameters, and the new Disposable return type is sub-classing the original return type, so code that already using this function in the vallina JS style can continue to work.

Having that level of drop-in compatibility means the unsafe nature of creating native resources also got inherited by the Disposable contract. People used to forget to call handler.close() can now forget to write using handle.

If that's the case, I don't really see the benifits of adding the using syntax. You can use DisposableStack + stack.use() + try block to do the same thing.

const stack = new DisposableStack();
try {
    const { read } = stack.use(disposableReader());
    const data = read();
    const { write } = stack.use(disposableWriter());
    write(data);
} finally {
    stack.dispose();
}

The syntactic benifits of using compared to this is trivial. And yet both doesn't enforce the user to properly dispose the resource.

If drop-in level backward compatibility is the "adoption risk" you are talking about, then the porposal made by @Jamesernator would also break the drop-in compatibility. Update: I'm wrong. @Jamesernator proposal actually maintains the drop-in compatibility. It's indeed a good solution.

It's not mutually exclusive to have Disposable type and DisposeScope and defer() to exist together. If you really need to maintain drop-in compatibility of your existing functions, sure you can adopt it to return a Disposable type. And it will keep having the unsafe nature of its old contract. But the DisposeScope and defer() contract is interoperable with Disposable without the using syntax. For example you can write a simple use() helper:

function use<T extends Disposable>(defer: Deferrer, disposable: uses T): T & used {
    defer(() => disposable[Symbol.dispose]());
    return disposable;
}

dispose {
    const { write } = use(defer, createHandle());
    write(data);
}

And you can maintain drop-in compatibility in this way. While new libraries can choose to adopt the defer contract to be safer.

Linter failure

Moreover, Ron's solution in enforcing correct usage of Disposable types rely heavily in type inference and linters. However, if you need to maintain drop-in compatibility when upgrading your existing resource creation function to Disposable, there would be a lot of problems to do linting.

For example, let's say you used to have these code:

type NativeHandle = {
    write(data: string): void;
    close(): void;
};
declare function createHandle(): NativeHandle;

const { write, close } = createHandle();
try {
    write(data);
} finally {
    close();
}

And you upgraded it to Disposable without changing how your code uses the new handle, then a linter would fail to properly check for Disposable usage:

type DisposableHandle = {
    write(data: string): void;
    close(): void;
    [Symbol.dispose](): void;
};
declare function createHandle(): DisposableHandle;

const { write, close } = createHandle();
//                       ^^^^^^^^^^^^^^ - return value is unused
try {
    write(data);
} finally {
    close();
//  ^^^^^^^ - linter cannot tell close() is using the handle
}
// linter error: createHandle() value is unused

TypeScript can probably have another type annotation syntax on the close() method to tell the type engine it will dispose the parent object, but I'm not sure how versatile it can be for all possible use cases. It could become a follow up question for Ron.

zaygraveyard commented 1 year ago

First off, thank you for the detailed replay.😄

As I understand it, the adoption risk comes from the fact that adopting your proposal required a breaking change to an existing API (ie. the change to the createHandle() function to accept a defer argument in case 2).

Example

Setup

Say I'm an author/maintainer of a library that offers createHandle()

function createHandle() {
  const { write, closeWriter: close } = nativeWriter();
  return { write, close };
}

The users of that library are currently required to call close() when they are done using it and they have the following code (from your example):

function main() {
  const { write, close } = createHandle();
  try {
    write(data);
  } finally {
    close();
  }
}

Now, that library could very much benefit from this proposal for resource management, so I wish to adopt it.

Adopting Ron's proposal

I just need to add a [Symbol.dispose]() to the returned object and tell the users that the library now supports the using keyword.

Note: This is a minor change, the users don't need to change anything in their code. But they can start using using if and when they want.

Adopting your proposal

I need to add a defer argument to createHandle() and tell the users that the library now requires the use in a dispose scope (or similar).

Note: This is a major/breaking change, the users are required to change all the createHandle() call sites, their current code would no longer work.

Conclusion

Adopting Ron's proposal is easier, requires less changes to users' code, than yours.

I hope this clarifies the "adoption risk" your proposal entails.

I'm not saying that Ron's proposal is better than yours, just that his is less of an adoption risk than yours.

@Jamesernator's proposal

After looking at it more closely, it would also require a breaking change to adopt (changing the return type of createHandle() instead of extending it) 😔.

rixtox commented 1 year ago

As I understand it, the adoption risk comes from the fact that adopting your proposal required a breaking change to an existing API (ie. the change to the createHandle() function to accept a defer argument in case 2).

No, adopting my proposal DOESN'T REQUIRE a breaking change to an existing API. You only adopt the defer signature when you don't have the compatibility restriction. You can still adopt returning Disposable for your existing APIs though, it doesn't conflicts with my proposal.

Adopting your proposal

I need to add a defer argument to createHandle() and tell the users that the library now requires the use in a dispose scope (or similar).

Note: This is a major/breaking change, the users are required to change all the createHandle() call sites, their current code would no longer work.

@zaygraveyard Please read the "Backward compatibility" section in my previous response. You can still adopt Disposable if you want drop-in compatibility without making any change to existing code. My proposal doesn't conflict with adopting Disposable. I showed a simple use() helper can make interoperating defer and Disposable very easy.

So in your example, you can adopt the Disposable return type sure:

function createHandle() {
  const { write, closeWriter: close } = nativeWriter();
  return { write, close, [Symbol.dispose]: close };
}

And your old code would work without any change:

function main() {
  const { write, close } = createHandle();
  try {
    write(data);
  } finally {
    close();
  }
}

But if you are writing any new code using my proposal, you can choose to use the use(defer, disposable) helper function to make it easier to write:

function use<T extends Disposable>(defer: Deferrer, disposable: uses T): T & used {
    defer(() => disposable[Symbol.dispose]());
    return disposable;
}

dispose {
    const { write } = use(defer, createHandle());
    write(data);
}

And even better if you are in a position to author new library functions, or your old function is internal and you are able to change its signature, then you can adopt the defer function signature for the most safeness benefits:

function createSafeHandle(defer: Deferrer) {
  const { write, closeWriter: close } = nativeWriter();
  defer(() => close());
  return { write };
}

dispose {
    const { write } = createSafeHandle(defer);
    write(data);
}

So I'm not objecting to Disposable interface, but I'm objecting to not having an option for writing new functions with a safer signature.

With Ron's proposal you only have the option to maintain compatibility sacrificing safety. With my proposal you can choose the compatibility approach if you have adoption restrictions. Or you can choose the safer approach if you don't need to maintain that level of compatibility.


So let me summarize my opinions on Ron's (@rbuckton) proposal:

  1. It's fine to have a Disposable interface to act like a standard for returning dispose method together with the resource. It defines a unified interface for disposing resources, enabling us to write common utilities to do explicit resource disposal. This includes DisposableStack.use(), or using, and also the use(defer, disposable) helper I introduced above.
  2. The Disposable interface can make upgrading existing functions possible without breaking any existing code, but in order to maintain that level of compatibility the contract would inherit the unsafe nature of the old contract, and it makes linting for correct Disposable usage even harder. (See "Linter failure" section in my previous response)
  3. The using syntax is not really necessary as you can do the same with DisposableStack + try block (shown in previous response). The only meaningful benefit I can see with using is when using with AsyncDisposable, it implicitly await the disposal making it slightly safer in case someone could forget to await on calling an async dispose function. But my proposal of a dedicated syntax to create a new dispose scope can also have that benefit.
  4. Although the using syntax is equivalent to using DisposableStack + try block, it doesn't give you any control on the implicit dispose scope it creates - it's purely bound to the enclosing {} code block. This is the reason in many cases you need to create an explicit DisposableStack and call stack.move() a lot when your resource needs to be passed to outside the enclosing {} block scope.

And to summarize how my proposal is improving these situations:

  1. As shown above, you can continue to adopt returning Disposable for your existing functions even when using my proposal. It serves as a way to provide a unified interface for explicitly disposing resources without breaking any existing code.
  2. If you are not in a position to change your existing function signature, sure adopt Disposable. It gives you the drop-in level compatibility without changing any existing code. And my defer contract can easily integrate with Disposable with a simple use(defer, disposable) helper. However, if you are a new library author, or just writing new functions, you can definitely choose to accept defer as a function argument to get a safer calling contract for creating resources. You will have the choice of compatibility or safeness with my proposal. While Ron's proposal only emphasizes on compatibility without a good solution to opt for safeness when possible.
  3. As shown before, with my proposal we don't need a using syntax - you can integrate Disposable with a simple use(defer, disposable) helper instead. However, I feel it's more valuable to have a syntax to create a new dispose scope and await on dispose when leaving the scope.
  4. A dispose scope as I proposed, can provide you with a defer handle you can pass down to call stack, making it much easier to move your resource outside your enclosing {} code block.

Again, I want to quote the design principle in Ron's own words. And I believe what I proposed adheres to his own guiding principle more closely than what he proposed.

  1. We want the API to guide the user towards the safest and most reliable avenue for resource cleanup by making those use cases the easiest to address.

My proposal can "guide the user towards the safest and most reliable avenue for resource cleanup" by recommending a calling convention of passing defer handle down the call stack.

  1. We want to allow the user to do less safe things for specific cases, but there should be more resistance on that path to ensure the average use case stays on (1).

My proposal "allows the user to do less safe things" when they have compatibility restrictions, "but there should be more resistance on that path":

So developers would be encouraged to use the safer defer approach when authoring new functions, "to ensure the average use case stays on (1)".

zaygraveyard commented 1 year ago

Thanks again for the detailed explanation.

Apologizes, it appears I had misunderstood your "Backward compatibility" section, but it's clear to me now that your proposal doesn't require a breaking change. So I believe it shouldn't have a big adoption risk.

Unfortunately, it might be already too late to change anything, since this proposal has reached stage 3.

benjamingr commented 1 year ago

Not speaking for Node.js as a whole but as a person who's currently part of the effort to adopt it for Node.js:

I believe that runtimes will choose to adopt Disposable wrappers for most native handles, such as the NodeJS fs.promises.FileHandle,

We have and we do support it (pretty recently). We are also very much in favor of this proposal and the deterministic aspect of it.

Honestly if I had to make cleanup opt out I would likely use a disposer pattern ala https://stackoverflow.com/questions/28915677/what-is-the-promise-disposer-pattern .

and they could easily leverage FinalizationRegistry for those wrappers

We have no interest in these I think? As mentioned by others this has to be deterministic to be useful, we haven't had good experience with solutions relying on GC. I do intend to use a lint rule like I do with forgetting await in my own code.

rixtox commented 1 year ago

@benjamingr Not speaking for Node.js as a whole but as a person who's currently part of the effort to adopt it for Node.js:

I believe that runtimes will choose to adopt Disposable wrappers for most native handles, such as the NodeJS fs.promises.FileHandle,

We have and we do support it (pretty recently). We are also very much in favor of this proposal and the deterministic aspect of it.

Honestly if I had to make cleanup opt out I would likely use a disposer pattern ala https://stackoverflow.com/questions/28915677/what-is-the-promise-disposer-pattern .

and they could easily leverage FinalizationRegistry for those wrappers

We have no interest in these I think? As mentioned by others this has to be deterministic to be useful, we haven't > had good experience with solutions relying on GC. I do intend to use a lint rule like I do with forgetting await > in my own code.

Had a quick look at the disposer pattern. I think it's equivalent to what I'm proposing.

The disposer pattern is a way to couple a scope of code with owning the resource. By binding the resource to a scope we make sure it is always released when we're done with it and we can't easily forget to release it.

This is essentially the key idea of my proposal as well. So basically their example:

withResource(function(resource){
    return fnThatDoesWorkWithResource(resource); // returns a promise
}).then(function(result){
   // resource disposed here
});

Can be implemented using my proposal:

async function withResrouce<T>(cb: (resource: Resource) => Promise<T>): Promise<T> {
    async dispose {
        const { resource, close } = nativeResource();
        defer(() => close());
        return cb(resource);
    }
}

But keep in mind that this pattern only works if withResource() returns a Promise. It makes sense if both your callback and your teardown functions are asynchronous, but if the close() function, and your fnThatDoesWorkWithResource() are indeed synchronous, it would unnecessarily turn your resource into asynchronous too. You could of course return a closure instead:

function withResrouce<T>(cb: (resource: Resource) => T): <S>(cps: (result: T) => S) => S {
    return (cps) => {
        let result: T;
        dispose {
            const { resource, close } = nativeResource();
            defer(() => close());

            result = cb(resource));
        }
        return cps(result);
    };
}

withResource((resource => {
    return fnThatDoesWorkWithResource(resource); // doesn't return a promise
}))((result) => {
    // resource disposed here
});

But as you can see this pattern is 1. equivalent to my proposal, and 2. less simple to author, and 3. doesn't provide flexible interface to deal with asynchrony.


As a comparison, let me show you the flexibility in dealing with asynchrony with my proposal.

Case 1. Synchronous resource teardown

If your resource's close() function is synchronous, you would only need to accept a synchronous defer() function:

function createResource(defer: Deferrer): Resource {
    const { resource, close } = nativeResource();
    defer(() => close());
    return resource;
}

And you can use it in both synchronous and asynchronous callers:

function syncFunc() {
    dispose {
        const resource = createResource(defer);
        // use resource
    }
}
async function asyncFunc() {
    async dispose {
        const resource = createResource(defer);
        // use resource
    }
}

Case 2. Asynchronous resource teardown

If instead your resource's close() function is asynchronous, you have to declare an asynchronous defer() dependency:

function createResource(defer: AsyncDeferrer): Resource {
    const { resource, close } = nativeResource();
    defer(async () => await close());
    return resource;
}

You can still use it in syncronous function if the defer comes from even a higher level caller:

function intermediateSyncFunc(defer: AsyncDeferrer): Resource {
    const resource = createResource(defer);
    // do something with resource
    return resource;
}

But eventually the defer() needs to come from some async scope:

async function asyncFunc() {
    async dispose {
        const resource = createResource(defer);
        // use resource
    }
}

I hope these examples can show that my proposal doesn't pollute intermediate synchronous functions to force them to become async. As a resource author, you only need to declare the dependency of synchronous or asynchronous defer().

benjamingr commented 1 year ago

Thanks for the response and sorry for being unclear. My points are:

Having something that is a resource does not imply to me "my expectation is for you to consume it as it only lives in this scope". There are cases we want this (for which the disposer pattern works) and cases we want await using resource = getResource() or for the user to const resource = getResource(); setTimeout(() => resource[Symbol.asyncDispose](), 1000) or a number of other things.

The key is that for our resources we can't always tie them to a lexical scope. We often do but not always.

Let's take an example: I have a disposable request stream whose lifetime needs to be bound to that of an HTTP request lifecycle in an HTTP server.

With this proposal users presumably wouldn't want to await using it since the framework takes care of the lifetime itself. They can do it to make sure the request is closed when the method is finished but they likely won't. The current proposal enables this use case quite nicely and has precedent in other programming languages.

I suspect your proposal may make it too easy to make errors because of its implicitness:

fastify.post("/someRoute", async (request, reply) => {
  async dispose {
    const body = traceAndValidateBody(request); // the request body should not be closed here but will be
    const items = fs.promises.open("/someRouteData"); // this file should be clsoed here
    // do stuff with file and body 
  }
});

We need the explicitness and determinism for this feature to be useful to us. I'm not saying what you're suggesting is bad and it's certainly interesting I just don't think it meets our requirements.

rixtox commented 1 year ago

@benjamingr Thanks for the response and sorry for being unclear. My points are:

  • What you are suggesting is indeed already possible with the disposer pattern. It's useful on its own
  • I want a way to say "here is a resource" and leave dealing with it to the user so they can await using it or decide to pass it down or whatever their code needs.

Having something that is a resource does not imply to me "my expectation is for you to consume it as it only lives in this scope". There are cases we want this (for which the disposer pattern works) and cases we want await using resource = getResource() or for the user to const resource = getResource(); setTimeout(() => resource[Symbol.asyncDispose](), 1000) or a number of other things.

I get what you are saying here. This is indeed a problem I have considered. But I believe this is just a type of mindset you need to adjust as a developer. I code a lot in Golang so this is already a familiar pattern for me to write code like:

func main() {
    f := os.Open("file")
    defer f.Close()

    var wg sync.WaitGroup
    defer wg.Wait()

    wg.Add(1)
    go func(){
        defer wg.Done()
        w.Write(data)
    }()
}

So I don't see why we can't do the same in JavaScript:

async dispose {
    const resource = getResource(defer);
    const wg = WaitGroup();
    defer(() => wg.wait());
    wg.add(1);
    setTimeout(() => {
        try {
            // do something with resource
        } finally {
            wg.done();
        }
    }, 1000);
}

Instead of reasoning about the exact point of disposing your resource, think about when you should extend the lifetime of your resource scope. I also code a lot in ANSI C so I have exposure to both mindsets, and I can tell the Go-style reasoning is much easier.

For example the following would be harder to reason in the other way but very easy to do with scope extension:

async dispose {
    const resource = getResource(defer);
    const wg = WaitGroup();
    defer(() => wg.wait());
    wg.add(1);
    setTimeout(() => {
        try {
            // do something with resource
        } finally {
            wg.done();
        }
    }, 1000);
    wg.add(1);
    setTimeout(() => {
        try {
            // do something with resource
        } finally {
            wg.done();
        }
    }, 1000);
}

I suspect your proposal may make it too easy to make errors because of its implicitness:

fastify.post("/someRoute", async (request, reply) => {
  async dispose {
    const body = traceAndValidateBody(request); // the request body should not be closed here but will be
    const items = fs.promises.open("/someRouteData"); // this file should be clsoed here
    // do stuff with file and body 
  }
});

I'm not sure if I understand correctly with this example of yours. Are you saying the items are not properly closed here? It can be corrected if you write:

fastify.post("/someRoute", async (request, reply) => {
  async dispose {
    const body = traceAndValidateBody(request); // the request body should not be closed here but will be
    const items = fs.promises.open("/someRouteData"); // this file should be clsoed here
    defer(() => items.close());
    // do stuff with file and body 
  }
});

Are you saying people would make assumptions that all resources can be automatically disposed if they put it in a dispose scope even it's a legacy resource that needs explicit disposal? If that's what you saying, maybe we can have a better name than dispose {} to avoid that confusion. But besides that I'm not seeing other significant problems with your example above.

rixtox commented 1 year ago

This thread has become too long, and what I'm proposing has improved several times. So I deem it necessary to create this mega post to be updated with the latest shape of my proposal, and gather FAQs from separate discussions to make it more accessible for everyone.

0. Motivation

The using syntax is merely a syntactic sugar for creating a try ... finally block and call the dispose method in the finally clause. It's not improving the unsafe natural of explicitly calling dispose method. You may leak (forget to using), excess dispose (using multiple times), and use-after-free with using, just like you would with explicitly disposing resources.

function createHandle(): Handle {
  const { write, close } = nativeWriter();
  return { write, close, [Symbol.dispose]: close };
}

type Handle = {
    write(data: string): void;
    close(): void;
    [Symbol.dispose](): void;
};

function leak() {
    const { write } = createHandle(); // oops, forgot to using
    write('data');
}

function useAfterFree() {
    using { write } = createHandle();
    setTimeout(() => {
        write('data'); // already gone
    }, 1000);
}

function excessDispose() {
    using handle = createHandle();
    array.forEach(writeWithHandle);
    function writeWithHandle(data) {
        using { write } = handle; // should not take ownership
        write(data);
    }
}

We should provide a safer semantics for the using syntax as an opt-in option.

1. The Proposal

I'm now in favor of @Jamesernator's proposal. Basically we introduce a new interface type:

type Creator<T> = {
    [Symbol.create](): T;
};

And change the semantics of using to:

{
    using x = expr;
    // typeof (expr) extends null | undefined | Disposable | AsyncDisposable
    //      | Creator<null | undefined | Disposable | AsyncDisposable>;
    // typeof x = typeof (expr) extends null | undefined
    //      ? typeof (expr)
    //      : typeof (expr) extends Disposable<infer T>
    //          ? T
    //          : typeof (expr) extends AsyncDisposable<infer T>
    //              ? T
    //              : typeof (expr) extends Creator<infer T extends null | undefined>
    //                  ? T
    //                  : typeof (expr) extends Creator<Disposable<infer T>>
    //                      ? T
    //                      : typeof (expr) extends Creator<AsyncDisposable<infer T>>
    //                          ? T
    //                          : never;
}

// transpiles to
{
  const $$try = { stack: [], error: undefined, hasError: false };
  try {
    ... // (1)

    let $$value = expr1;
    if ($$value !== null && $$value !== undefined) {
      const $$create = $$value[Symbol.create];
      if (typeof $$create === "function") {
        $$value = $$create();
      }
    }

    const x = $$value;
    if (x !== null && x !== undefined) {
      const $$dispose = x[Symbol.dispose];
      if (typeof $$dispose !== "function") {
        throw new TypeError();
      }
      $$try.stack.push({ value: x, dispose: $$dispose });
    }

    ... // (2)
  }
  catch ($$error) {
    $$try.error = $$error;
    $$try.hasError = true;
  }
  finally {
    while ($$try.stack.length) {
      const { value: $$expr, dispose: $$dispose } = $$try.stack.pop();
      try {
        $$dispose.call($$expr);
      }
      catch ($$error) {
        $$try.error = $$try.hasError ? new SuppressedError($$error, $$try.error) : $$error;
        $$try.hasError = true;
      }
    }
    if ($$try.hasError) {
      throw $$try.error;
    }
  }
}

2. Adoption

@Jamesernator's proposal is fully compatible with @rbuckton's proposal. So all the adoption approaches of @rbuckton's proposal are the same. Meanwhile it provides a safer alternative convention:

function createHandle(): Creator<Disposable<Handle>> {
    return { [Symbol.create]() {
        const { write, close } = nativeWriter();
        return { write, [Symbol.dispose]: close };
    } };
}

{
    using { write } = createHandle();
    write('data');
}

It would also makes sense to change DisposableStack to adopt the Symbol.create contract too. @rbuckton's design of DisposableStack is for it to be an affine type, which means it should be consumed by using, and only use move() to move it onto other scopes.

FAQ

Why the Symbol.create contract is safer?

I'd refer to @Jamesernator's comment for the answer. It effectively prevents innocent mistake of forgetting to using, thus safer against leaks. Use after free and excess dispose can still happen but they can be address with some lifetime management paradigms or calling conventions.

But it's too late, we are already at stage 3

Yes, this is unfortunate indeed. I really wish it got more attention at earlier stages. Even Ben Lesh, the author of arguably the largest resource/async management library, RxJS, only heard of this proposal only after stage 3. There's clearly a gap between the spec committee and the developer community.

I don't know if there has been precedence of revisiting a stage 3 proposal's design, but this feature is so important that I believe is worth of the trouble to make sure we do it right. Especially given the fact that it lacks community feedback in the early stages.

If you also think this proposal deserves a better look, please also raise your voice to the committee.

Archived Proposal (7/5/2023): Dispose scope and defer contract > > > Reason for archive: This proposal recommends removing `using` syntax in favor of creating scope with `dispose {}` syntax. This change is unlikely to be accepted into a stage 3 proposal. [@Jamesernator's proposal](https://github.com/tc39/proposal-explicit-resource-management/issues/159#issuecomment-1613170795) is an add-on to @rbuckton's proposal. It's more likely to be accepted. Although it alone still cannot prevent excess dispose, but [@mhofman pointed out](https://github.com/tc39/proposal-explicit-resource-management/issues/159#issuecomment-1622554333) we can build safer contract like the `defer` contract on top of `Symbol.create`. ## 0. Motivation The `using` syntax is merely a syntactic sugar for creating a `try ... finally` block and call the dispose method in the finally clause. It's not improving the unsafe natural of explicitly calling dispose method. You may leak (forget to `using`), excess dispose (`using` multiple times), and use-after-free with `using`, just like you would with explicitly disposing resources. ```ts function createHandle(): Handle { const { write, close } = nativeWriter(); return { write, close, [Symbol.dispose]: close }; } type Handle = { write(data: string): void; close(): void; [Symbol.dispose](): void; }; function leak() { const { write } = createHandle(); // oops, forgot to using write('data'); } function useAfterFree() { using { write } = createHandle(); setTimeout(() => { write('data'); // already gone }, 1000); } function excessDispose() { using handle = createHandle(); array.forEach(writeWithHandle); function writeWithHandle(handle) { using { write } = handle; // should not take ownership write('data'); } } ``` The syntactic benefit from `using` compared to `DisposableStack` + `try ... finally` is trivial: ```ts const stack = new DisposableStack(); try { const { read } = stack.use(createHandle()); const data = read(); const { write } = stack.use(createHandle()); write(data); } finally { stack.dispose(); } ``` We should remove the `using` syntax from the current proposal, and find a better syntactic feature that can provide a safer resource management convention. ## 1. The Proposal Resources should be bound to some dispose scope at creation, and be disposed when leaving the scope. The basic form of my proposal involves two things: 1. Create a new dispose scope. (`DisposeScope()`) 2. Defer cleanup from the dispose scope. (`defer()`) ```ts DisposeScope((defer: Deferrer) => { const { write, close } = nativeHandle(); defer(() => close()); write('data'); }); // handle is closed here ```
Type Definitions ```ts declare function DisposeScope( task: Return extends PromiseLike ? never // inner function has to be synchronous : (defer: Deferrer, ...args: Args) => Return, ...args: Args ): Return; type Deferrer = { ( teardown?: T extends (error?: DyingError) => PromiseLike ? never // teardown has to be synchronous : T ): void; }; type Teardown = (error?: DyingError) => T; ```
And to support asynchronous teardown, there's also an async dispose scope. ```ts await AsyncDisposeScope((defer: AsyncDeferrer) => { const { writeAsync, closeAsync } = nativeAsyncHandle(); defer(async () => await closeAsync()); await writeAsync('data'); }); // handle is closed here ```
Type Definitions ```ts declare async function AsyncDisposeScope( task: (defer: AsyncDeferrer, ...rest: Args) => Return, ...args: Args ): Promise>; export type AsyncDeferrer = { (teardown?: Teardown): void; }; ```
In addition, to make it easier to create the dispose scope, and make it safer in case people forget to await on the async dispose scope, I propose to have a dedicated syntax for creating dispose scope: ```ts dispose { const { write, close } = nativeHandle(); defer(() => close()); write('data'); } // transpiles into DisposeScope((defer: Deferrer) => { const { write, close } = nativeHandle(); defer(() => close()); write('data'); }); ``` And for the async version it also implicitly await on the dispose scope: ```ts async dispose { const { writeAsync, closeAsync } = nativeAsyncHandle(); defer(async () => await closeAsync()); await writeAsync('data'); } // transpiles into await AsyncDisposeScope((defer: AsyncDeferrer) => { const { writeAsync, closeAsync } = nativeAsyncHandle(); defer(async () => await closeAsync()); await writeAsync('data'); }); ``` ## 2. Adoption The basic forms above allow resource to be cleaned up explicitly by calling `defer()` to schedule the cleanup. This works well with all existing library functions. For example: ```ts import { open } from 'node:fs/promises'; async dispose { const filehandle = await open('thefile.txt', 'r'); defer(() => filehandle.close()); } ``` As a library maintainer, **YOU CAN DO ABSOLUTELY NOTHING**: you don't have to upgrade your existing APIs to support this proposal. Old code doesn't use the `dispose {}` syntax can continue to work while new code that choose to adopt `dispose {}` can schedule cleanup as shown above. The adoption can happen completely at consuming side. This gives the proposal maximum compatibility. However, as a library maintainer, you **MAY** choose to do something to improve the usability and safeness when creating resources with your APIs. In one case, if you are a maintainer of an existing library, you absolutely cannot break anything in production, meaning you cannot have incompatible change on existing APIs. Then you would have two choices: 1. Do it in a fully-compatible way for usability but not for safety. 2. Create new APIs, similar to the fork of `node:fs` and `node:fs/promises`, and adopt a safer calling convention. ### 2.1 Adoption with Backward Compatibility The 1st approach can be done with the `Disposable` type from Ron's (@rbuckton) proposal. It can maintain maximum compatibility while providing a unified disposal interface making it slightly easier to write code with legacy handles without introducing new APIs. However, it doesn't provide the maximum level of safety guarantee compared to the 2nd approach. Therefore, people should be encouraged to adopt the 2nd approach when possible. ----
Disposable type, and use(defer, disposable) helper instead of using syntax Optionally, if you want to "standardize" your resource cleanup in a backward-compatible way. Meaning you are not at a position to change your function's signature, but want people using the `dispose {}` syntax to use your resource even easier, you can choose to adopt the `Disposable` interface for your function's return value. As long as your return value is an object, extending it to the `Disposable` interface is merely a subclass, which means fully compatible with existing code. But the resource will get a unified interface for disposal, meaning we can write standard utility for scheduling that disposal inside `dispose {}` scope. For example, if you used to have legacy function like: ```ts function createHandle(): Handle { const { write, close } = nativeWriter(); return { write, close }; } type Handle = { write(data: string): void; close(): void; }; ``` You can extend the `Handle` type into a `Disposable` interface: ```ts function createHandle(): Handle { const { write, close } = nativeWriter(); return { write, close, [Symbol.dispose]: close }; } type Handle = { write(data: string): void; close(): void; [Symbol.dispose](): void; }; ``` Existing code using the legacy return type can continue to work: ```ts function main() { const { write, close } = createHandle(); try { write(data); } finally { close(); } } ``` But for new code using the `dispose {}` syntax, we can introduce a standard helper `use(defer, disposable)` to make it slightly easier to schedule cleanup of `Disposable` resources. ```ts function use(defer: Deferrer, disposable: uses T): T & used { defer(() => disposable[Symbol.dispose]()); return disposable; } dispose { const { write } = use(defer, createHandle()); write(data); } ``` This means we don't need a dedicated `using` language level syntax.
---- ### 2.2 Adoption with Safer Guarantees However, if you are authoring new library, writing new functions, or just refactoring internal code, and able to define or change your function's signature, it's recommended to go with the 2nd approach to adopt the `defer` contract to get safer guarantees. The `defer` contract is simple: **If your function delegates cleanup to the caller, then accept a `defer` argument in your function's signature.** For example: ```ts function createHandle(defer: Deferrer, name: string): Handle { const { write, close } = nativeHandle(name); defer(() => close()); return { write }; } // to use it, create a dispose scope dispose { const { write } = createHandle(defer); write('data'); } ``` If your resource requires asynchronous cleanup, use the `AsyncDeferrer` instead: ```ts function createAsyncHandle(defer: AsyncDeferrer, name: string): Handle { const { writeAsync, closeAsync } = nativeAsyncHandle(name); defer(async () => await closeAsync()); return { writeAsync }; } // to use it, create an async dispose scope async dispose { const { writeAsync } = createHandle(defer, 'name'); await writeAsync('data'); } ``` Adopting `AsyncDeferrer` as a parameter doesn't "pollute" your `createAsyncHandle` to become async as well, as long as your resource creation is synchronous. In fact, intermediate function that synchronously does something with your resource, but returns the resource to its caller can also stay synchronous: ```ts function intermediateFunc(defer: AsyncDeferrer, name: string): Handle { const handle = createHandle(defer, name); // synchronously do something with the handle return handle; } ``` ### 2.3 Extending Dispose Scope You may quickly realize that declaring dispose scope to be bound to a block scope makes it difficult to "move" your resource outside that scope, and onto some asynchronous execution context. For example: ```ts async function main() { async dispose { const { write } = createHandle(defer, 'name'); setTimeout(async () => { await write('data'); // use after free }, 1000); button.addEventListener('click', () => { await write('data'); // use after free }, { once: true }); } } ``` This can be solved by extending the lifetime of the dispose scope to cover the async contexts. There can be many solutions to this problem, but I would like to borrow the `WaitGroup` straight from Golang, which is also the language the `defer` contract was inspired from.
WaitGroup implementation ```ts async function main() { async dispose { const { write } = createHandle(defer, 'name'); const wg = WaitGroup(); defer(() => wg.wait()); wg.add(1); setTimeout(async () => { try { await write('data'); // safe } finally { wg.done(); } }, 1000); wg.add(1); button.addEventListener('click', () => { try { await write('data'); // safe } finally { wg.done(); } }, { once: true }); } } ```
```ts async function main() { async dispose { const { write } = createHandle(defer, 'name'); const wg = WaitGroup(); defer(() => wg.wait()); wg.add(1); setTimeout(async () => { try { await write('data'); // safe } finally { wg.done(); } }, 1000); wg.add(1); button.addEventListener('click', () => { try { await write('data'); // safe } finally { wg.done(); } }, { once: true }); } } ``` Developers need to build up a mindset to reason about **when they need to extend the lifetime of a dispose scope,** instead of when they need to explicitly call dispose. This change in mindset would actually make development much easier. If you are not convinced, just try to find an exact point you can explicitly call dispose in the above example. ## FAQ ### Why the `defer` contract is safer than `Disposable`? As a function author, you lose control once your function returns. If you return the dispose method together with the resource in a `Disposable` type, you lose control on how the resource should be disposed. Yes you would grant the caller the maximum power in controlling the resource's lifetime, but great power comes with great responsibility too. The caller may forget to dispose (leak), it may dispose more than once, it may use-after-free. To prevent these mistakes, the caller needs to impose some kind of self-discipline to perform proper lifetime management. Linter can help in some cases, but it's likely to produce false positives. The `defer` contract imposes an opinionated discipline in the function's signature. It makes sure: 1. `defer` must come from some dispose scope. 2. Teardown scheduled with `defer()` must be executed once and only once. 3. The caller must be aware of the deferred teardown because it explicitly passed `defer` as argument. This discipline works as long as your code can be type checked. And it already solved two issues: leak, and excess dispose. Use-after-free still needs developer's attention, but it's relatively easy: extend the lifetime of the resource scope when it's needed. I've demonstrated how to do it with `WaitGroup`, but there can be many ways to make it simple.
benjamingr commented 1 year ago

If you also think this proposal deserves a better look, please also raise your voice to the committee.

I mean, your proposal is interesting but I'm less interested in it for Node for the aforementioned reasons above and I am very interested in the current proposal since I believe it makes the right tradeoffs and meets our needs.

It's easy to implement defer on top of, the disposer pattern works for "opt out" etc. This proposal is 6 years old with the same champion and has in principle been the same since then with the biggest difference being using(disposable) {} as a block vs. the current way.

rixtox commented 1 year ago

@benjamingr I mean, your proposal is interesting but I'm less interested in it for Node for the aforementioned reasons above and I am very interested in the current proposal since I believe it makes the right tradeoffs and meets our needs.

Can you clarify your "aforementioned reasons" a bit for me? Didn't my response address your "resource may not tie to lexical scope" concern?

I just want to understand the reasoning behind your opinions. I assume you come from the perspective of an author of Node.js standard librariy APIs, and I can see you have been adding Disposable interface to a lot of APIs in Node.js. So I assume what you mean by "right tradeoffs and meets our needs" boils down to compatibility requirements. I'm not objecting to using Disposable interface at all. I think it's fine for existing libraries to add it to their resource types. It would provide a unified interface for performing teardown.

I think the reason why you are "less interested" in what I'm proposing also boils down to the same perspective: you are not authoring new libraries or APIs, so a breaking change in function signature to gain more safety guarantee is not a viable option. Which is also why you are not out right objecting to what I'm proposing.

It's easy to implement defer on top of, the disposer pattern works for "opt out" etc.

If we allow a using syntax to be implemented, it would be very hard to get a defer mechanism in the language later, because you would have two different mechanisms for scheduling teardown, both tied to some lexical scope but differs in semantics. At this point I think it's best to continue the current proposal with Disposable and DisposableStack, but remove the using syntax from the current proposal. This means integrators like you can continue your work, and developers would need to write stack.use() to consume those resources for the moment, making it slightly more verbose than the using syntax. But it would give us a chance to find a better way to write code that requires teardown, in a follow-up proposal.

mhofman commented 1 year ago

@rixtox, thanks for the summary above.

If I understand it right, your main concern is you want to support explicit resource management at creation instead of acquisition? In your proposal it's accomplished by passing a Deferrer param to the resource makers.

Like @benjamingr I believe you can accomplish this today with the current proposal and DisposableStack. To take your example:

    async function main() {
        using stack = new AsyncDisposableStack();
        const defer = stack.defer.bind(stack);
        const { write } = createHandle(defer, 'name');
        const wg = WaitGroup();
        defer(() => wg.wait());

        wg.add(1);
        setTimeout(async () => {
            try {
                await write('data'); // safe
            } finally {
                wg.done();
            }
        }, 1000);

        wg.add(1);
        button.addEventListener('click', () => {
            try {
                await write('data'); // safe
            } finally {
                wg.done();
            }
        }, { once: true });
    }

You can also pass stacks around in makers instead of a simple defer, which can call stack.use on resources they allocate. You can also do things like newStack.use(oldStack.move()) if you want to transfer the ownership of the stack to extend the lifetime of your resources beyond a simple scope. In that sense the stack object is similar to your DisposableScope.

rixtox commented 1 year ago

@mhofman

If I understand it right, your main concern is you want to support explicit resource management at creation instead of acquisition? In your proposal it's accomplished by passing a Deferrer param to the resource makers.

Like @benjamingr I believe you can accomplish this today with the current proposal and DisposableStack. To take your example:

// ...
using stack = new AsyncDisposableStack();
// ...

It's not the same. The AsyncDisposableStack still subjects to the risk of forget to using it. My main concern is the using syntax doesn't improve the safeness, and the current proposal doesn't provide even an opt-in solution for a safer resource management convention.

mhofman commented 1 year ago

The same risk exist if the user writes:

        const { write } = createHandle(() => {}, 'name');

Or simply not call defer in the "dispose" blocks.

The using syntax is about explicitly opting into automatic resource management based on lexical scope. People can forget to call defer just as much as they can forget to write using.

You can opt into extending the explicit resource management to creation time by adopting the convention you suggest of creation time parameter, nothing in this proposal prevents that. However, I am personally not convinced this is really needed.

Btw, the proposal has moved away from explicit "dispose" blocks as they are not ergonomic. Furthermore, defer is a valid identifier in JS, so you can't simply have a dispose block introduce a "hardcoded" defer identifier.

rixtox commented 1 year ago

@mhofman The same risk exist if the user writes:

        const { write } = createHandle(() => {}, 'name');

This is not an unintentional mistake people can make. You can only write this code knowing that you need to pass a defer argument to the function, and you intentionally choose to give it a void function instead of a valid defer function. This is way more explicit and exoteric than an innocent mistake of forgetting to using. I would not call it a "risk" at all.

That being said, we can make this misuse very difficult by Symbol-tagging the Deferrer type so only the defer function created by a dispose scope can be passed to the creation function, and misuse like this can be caught by type checker.

Or simply not call defer in the "dispose" blocks. The using syntax is about explicitly opting into automatic resource management based on lexical scope. People can forget to call defer just as much as they can forget to write using.

You only need to call defer if you are authoring the resource creation function using the defer contract, or if you are integrating directly with legacy resource handles. You can't "forget to call defer" when using a resource wrapped with the defer calling contract, because as a resource consumer, you simply don't call defer at all - you pass it down to the creation function.

Convention Resource Producer Resource Consumer Leak Risk Excess Dispose Risk
using returns Disposable using disposable forget using Multiple using
defer accepts defer createResource(defer) only if you explicitly pass void defer function (not a risk) Not possible

The benifits of having the defer contract is, when your resource creation function is wrapped inside the defer contract, as a resource consumer, you can easily write safer code by creating a dispose scope, and less likely to have leak and excess dispose problems.

You can opt into extending the explicit resource management to creation time by adopting the convention you suggest of creation time parameter, nothing in this proposal prevents that.

As I mentioned above, using stack + stack.defer.bind(stack) doesn't provide the safety guarantee because the stack itself can have the same risks. The closest thing you can have without any change to the language syntax is DisposeScope from my summary post.

Furthermore, defer is a valid identifier in JS, so you can't simply have a dispose block introduce a "hardcoded" defer identifier.

The exact syntax of a dispose scope can be discussed more. I'm not a JS syntax expert, the dispose {} syntax I'm suggesting is only a direction.

rixtox commented 1 year ago

@mhofman If you are not a fan of the dispose {} syntax, maybe we can keep having a using keyword but change its semantics to:

{
    using x = expr1;
    using y = expr2;
}

// transpiles to
DisposeScope((defer) => {
    x = expr1(defer);
    y = expr2(defer);
});

And have a helper function Disposable.use(disposable)

class Disposable {
    static use<T extends Disposable>(disposable: using T): (defer: Deferrer) => used & T {
        return (defer) => {
            defer(() => disposable[Symbol.dispose]());
            return disposable;
        };
    }
}

Then you can consume Disposable values like this:

declare function createHandle(): { write: (data: string) => void, [Symbol.dispose](): void };
using { write } = Disposable.use(createHandle());

This would be closer to the "disposer pattern". So a safer contract for this is to return a disposer closure:

type Disposer<T> = {
    (defer: Deferrer): T;
};
type AsyncDisposer<T> = {
    (defer: AsyncDeferrer): T;
};
function createDisposerHandle(): Disposer<Handle> {
    return (defer: Deferrer) => {
        const { write, close } = nativeHandle();
        defer(() => close());
        return { write };
    };
}
function createAsyncDisposerHandle(): AsyncDisposer<AsyncHandle> {
    return (defer: AsyncDeferrer) => {
        const { writeAsync, closeAsync } = nativeAsyncHandle();
        defer(async () => await closeAsync());
        return { writeAsync };
    };
}
function syncFunc() {
    using { write } = createDisposerHandle();
    write('data');
}
async function asyncFunc() {
    using { writeAsync } = createAsyncDisposerHandle();
    await writeAsync('data');
}
If you don't mind the confusion, you can even make it compatible to the current `using` semantics. ```ts { using x = expr; } // transpiles to DisposeScope((defer) => { const val = expr; let x; if (isDisposable(val)) { defer(() => val[Symbol.dispose]()); x = val; } else if (isFunction(val)) { x = val(defer); } else { throw new TypeError('unexpected type'); } }); ``` But I would not recommend it as it's confusing.
mhofman commented 1 year ago

I think some of the "I forgot to use using" cases could be mitigated by the idea from https://github.com/tc39/proposal-explicit-resource-management/issues/159#issuecomment-1613170795, where a Symbol.create would force the consumer of the resource into using instead of const or let.

function DeferableStack() {
  const invalidUsageGuard = () => { throw Error('Invalid usage: must use `using`') };
  invalidUsageGuard[Symbol.create] = function() {
    const stack = new DisposableStack();
    const defer = stack.defer.bind(stack);
    defer[Symbol.dispose] = stack[Symbol.dispose].bind(stack);
    return defer;
  };
  return invalidUsageGuard;
}

Then any code can do

using defer = DeferableStack();