-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Masking disposable #204
Comments
I think your analysis is correct, and it wouldn’t be fully achievable through a Proxy because that would break internal slot/private field usage. Since this proposal is already stage 3 it wouldn’t be appropriate to add anything new to it, but i think the real answer is, if you have a thing with a capability and want to control the use of the capability, you just don’t give the thing to anyone else. |
hmm, ok, that constrains a few options I think but it makes sense. Thank you! |
IMO, while this proposal is about explicit resource management, it doesn't absolve the program from thinking about what scope owns a resource's lifetime. There is no explicit transfer or share semantics in JS, and this proposal does not introduce any. As such I think it remains necessary for functions to document whether they keep control or not of their return value, and whether they assume control or not of their arguments. |
You may also wish to take a look at #195, which suggests a mechanism by which the capability to close a resource could be separated from the resource itself. |
FWIW, the key use case I'm thinking about here are host-defined objects with a wrapping JS object and an internal underlying C++ object. Specifically, can we safely dispose of the underlying C++ object when I do think there are some potential footguns buried in here but some well crafted linting rules ought to be able to catch those. |
Another quick clarification, I assume the following is also true...
|
Correct. |
If your intent is to explicitly give ownership of a resource to another function, one option is to emulate class Foo {
#state;
...
[Symbol.dispose]() {
if (this.#state) {
const state = this.#state;
this.state = undefined;
cleanup(state);
}
}
move() {
const copy = new Foo();
copy.#state = this.#state;
this.#state = undefined;
return copy;
}
}
function handle(foo) {
using bar = foo;
}
{
using foo = new Foo();
handle(foo.move());
}
// or...
function handle(foo) {
// some logic that may throw before I call 'move()'...
using bar = foo.move();
}
{
using foo = new Foo();
handle(foo);
} If your intent is to not provide ownership, you can wrap your resource in a disposable container that is able to perform cleanup on your object: // Gives `FooContainer` access to private cleanup of `Foo`
let disposeFoo;
class Foo {
...
static { disposeFoo = foo => foo.#disposePrivate(); }
#disposePrivate() { ... }
}
class FooContainer {
#foo = new Foo();
get foo() { return this.#foo; }
[Symbol.dispose]() {
if (this.#foo) {
const foo = this.#foo;
this.#foo = undefined;
disposeFoo(foo);
}
}
}
{
using fooContainer = new FooContainer();
const foo = fooContainer.foo;
handle(foo);
} |
function foo() {
const xyz = bar();
return promise.then(() => {
return xyz;
},(e) => {
xyz[Symbol.dispose]();
return Promise.reject(e);
});
} @jasnell Shouldn't we leave |
Yes.
async function foo() {
using xyz = bar();
const result = await promise;
// xyx has not been disposed.
return result;
}
function foo() {
using stack = new DisposableStack();
const xyz = stack.use(bar());
... // any exceptions resulting from acquiring 'promise' result in disposal
const newStack = stack.move();
return promise.then(() => {
// 'stack' has been disposed, but since we moved its contents into 'newStack', 'xyz' has not yet been disposed
}).finally(() => {
using stack = newStack; // here we can use 'newStack' to hook back into lifetime management for 'xyz'
// 'xyz' is now disposed.
});
} One of the main purposes of |
Generally, yes, but ... Misbehaving Code :-) |
Really appreciate the responses, all super helpful. What I imagine from all of this is likely a new range of linter rules that help catch the problematic patterns and promote these correct approaches. |
Related to this topic - how does one avoid a double dispose call footgun? Is the expectation that dispose is always singular? Would it ever make sense for dispose to delete its own property (ie, having a call to Symbol.dispose make the object as dead using another symbol, or actually calling |
I generally wouldn't recommend deleting a method since that changes the object's shape (which can affect runtime performance). It also wouldn't prevent something like: {
using a = ...;
using b = a;
} // essentially disposes 'a' twice because Generally, it's better to track whether |
Good points. There's just a temptation to not need another slot, while I have come across some double call cases (specifically when wanting to support both a finalization registry and explicit resource management together). Perhaps a pattern of |
In most cases a disposable has at least one field that holds the actual resource to be disposed. You can almost always just use that field as the disposal indicator rather than declaring a new one, which is exactly what I'm showing in the |
@ljharb... my apologies if this has already been discussed and I just missed it, but what would be expected to happen in the following case:
As I understand it, the inner block will call the
Symbol.dispose
on the object instance, correct?What if that is passed to a function in a library and the caller is not aware that the library is making use of
using...
?Again, as I understand it, when the scope for
handle(...)
exits theSymbol.dispose
will be called.If that is correct, it may be worthwhile to provide a means of passing a disposable around in a way that masks the fact that it is disposable.
This is obviously possible to accomplish using a
Proxy
but if my understanding here is correct, might this be common enough that it is worth baking into the proposal?The text was updated successfully, but these errors were encountered: