Disposable is leaky by default #159
Comments
|
I made a proof-of-concept preliminary implimentation based on Node.js AsyncLocalStorage / AsyncResource and 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: Note that the resource registered to the async Also note that the resource returned by 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: Notice that even we enclosed second |
|
I'm not sure I follow what the suggestion is. You can already perform some implicit cleanup through 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. |
|
IMO forgetting to declare a disposable with |
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.
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.
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 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 |
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 |
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
Although execution context and resource context should be decoupled, they need to work together to make sure correct program behavior:
|
|
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. |
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.
That is not true. This is the first thing I mentioned how resource context should work:
The deallocation happens when the binding resource context closes.
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.
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. |
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.
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 }); |
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 hereWhere 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});
}));
}); |
|
In that example, if you fail to call 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. |
|
Also this approach is bound to the |
|
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. |
|
An exception in an event listener is not the only way to fail to call |
|
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. |
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. |
|
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? |
Since the asynchronous function involved is necessary resolved only by an explicit call to
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. |
|
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. |
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 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.
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
This can already be accomplished with
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 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;
});
}
This is my major concern for your proposal, and the reason I brought up The goal of
Ref counting isn't nearly as reliable here as you make it out to be. You can't count references statically because
There's no point in ref counting if the scope still can't survive past the
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.
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. |
|
@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 diagnoseThere 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.
I know I was the one first mentioned about Your response still doesn't address the undiagnosable nature of leaking by not 2. Sharing resource to multiple detached async callsI 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;
}
Calling async functions in detached mode (without using It is possible to do it imperatively with 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
I think you got a dilemma here:
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
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 capturingI avoided using the 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 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
Yes. This is the same way if you constructed a nested 4.1. Async resource context capturing and GCI 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. |
I believe that runtimes will choose to adopt 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 If most native handles have 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.
Yes, leaks are possible with For example, I've been considering a follow-on proposal to add
Intended resource lifetime can always vary based on need.
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 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.
This does not function the same way as
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.
No, this is not quite the same. A SummaryI'd like to summarize my position as to what we've discussed so far: Native implicit pass-down is not viableImplicit 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 Hosts and built-ins can and should guard against leaky native handlesYes, Linters and type checkers can help as wellThis can also be guarded against by linters and type checkers, even if only in part. For example, TypeScript might choose to implement a 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 exceptionsIt's important that a developer can reason over exceptions thrown during disposal. Both Resource management should be easy to useIf 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 |
1. Leak diagnose
I still think it's not an easy task to author safe
What you said here is not in alignment with what you previously said on another thread:
If you were being truthful to those doctrines, you would oppose to allowing
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.
|
If the perscriptive guidance for a
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.
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.
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. On the other hand, 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 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.
If you want your resource lifetime to be divorced from execution context, we already have As I see it, you have essentially suggested three things:
I believe that (1.a) and (2) can be handled by hosts implementing appropriate safe native handle wrappers that leverage 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 I believe that Finally, I don't see how using |
I see what you mean, and it's a valid point. I only considered a function declaring a
I just learned that Node.js 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 Probably something like |
This is most likely not an option. When A
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., This is where the 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. |
I'm sure many people found that
Instead of mutating the object passed to
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 |
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.
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.
// 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
}
}
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. |
|
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. |
|
The So the RHS expression can be a |
The
I would strongly oppose to returning
Your above code still works under the new proposal since it's compatible with Ron's current proposal. We should not make generator to inherently adopt the
Again, It's just the |
I see the create symbol as a way for the API to choose to enforce This is up to the creator to decide if they're ok with their disposable resources being used with plain |
Yes, to enforce
For those cases, they should return |
|
I've explicitly avoided completely emulating Python's semantics for One issue I have with adapting Python's approach is that shoehorning it into existing APIs in the DOM or NodeJS would mean those existing APIs would have to add something like I have concerns about a declaration form like I might be willing to consider an approach where something like // error, resource management protocol not implemented
using x = { };
// ok. object itself is the resource stored in 'x', and who's @@dispose is tracked.
using x = { [Symbol.dispose]() { } };
// ok. @@create is called to get the resource to store in 'x', who's @@dispose is then tracked
using x = { [Symbol.create]() { return { [Symbol.dispose]() { } }; } };
// ok...ish. but 'x' isn't the actual resource, you'd have to call @@create manually
const x = { [Symbol.create]() { return { [Symbol.dispose]() { } }; } };If most existing API's would have to implement Assuming we could get committee buy-in on |
|
That said, I don't think |
|
What do you think about something like this to enforce using of class File {
#fd;
#entered = false;
constructor(path) {
this.#fd = open(path);
}
read() {
if (!this.#entered) throw new Error("You must setup `using` before using the File");
return read(this.#fd);
}
close() {
close(this.#fd);
}
get [Symbol.dispose]() {
this.#entered = true;
return this.close;
}
}try {
let file1 = new File("a");
file1.read(); // throws
} catch {}
{
using file2 = new File("b");
file2.read();
} // closes the file |
|
Make |
|
@nicolo-ribaudo const file = new File("a");
try {
file.read(); // should be allowed but you would throw
} finally {
file.close();
}It makes the API not backward compatible. Even without the requirement of backward compatibility, making |
|
@rixtox How can you at the same time force someone to use In the example above you were using a |
I don't think we would require that const file = new File(...), dispose = file[Symbol.dispose];
try {
file.read();
}
finally {
dispose.call(file);
} |
A well-written disposable resource should guard against its methods being used in a manner inconsistent with its state in any case. In the |
Once the resource has left your control, the consumer will always have the flexibility to use it as they see fit, and adding it to a
Maybe, but not much weaker than a |
You can't. My point is if it would already make the returned object not backward compatible, why not just implement or adopt a proper calling convention like
Can you explain how it would overcomplicate
There's hardly any operations that can have a side effect of actually calling |
The example I linked explicitly does not use |
|
If Am I missing something? |
@rbuckton is this in reference to or ? I understand why |
I have the same question as @senocular. Your example can be written as: class AsyncPluginHost {
// ...
static async create() {
// ...
await using stack = new AsyncDisposableStack();
host.#disposables = stack.move();
// ...
}
// ...
async [Symbol.asyncDispose]() {
if (!this.#disposed) {
this.#disposed = true;
await using disposables = this.#disposables;
this.#socket = undefined;
this.#channel = undefined;
this.#disposables = undefined;
}
}
}It doesn't overcomplicate anything. And it works even if |
What is unfortunate here is that the user needs to introduce an otherwise unused binding This does make it harder for users to leverage |
|
I'd also like to point out that Python's def __enter__(self):
return selfand as such does not require the use of |
That's pretty common for RAII. e.g. in C++ we have
Using |
A recommendation is fine, but I don't think mandating this in the API is a necessity. A user may need to employ more complicated cleanup in a I do think the use of |
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
usingon the current scope, or add to someDisposableStack.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()andfree(), 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.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.
Maybe it's better to not exposing the dispose method on any returned value. A dispose method must be registered to some
DisposeScope. While decouplingDisposeScopefrom block scope, making it similar to Autorelease Pool Blocks in Objective-C. So we don't need explicit transfer of ownership when a function isusingsome Disposable resource but wants to delegate its disposal to a parentDisposeScope. Also aDisposeScopecan be stored and re-opened to support use cases like class.So it would look something like this:
Enclosing some code inside a
DisposeScopemeans you want the scope to collect all the resource dispose methods, and call them when theDisposeScopeis closed, and to make sure all dispose methods complete before executing the statements after theDisposeScopeblock.You can even take it further to not pass
DisposeScopeas an argument. Instead making it an environment slot on the execution context. Adeferkeyword can be added to register a disposal method to the enclosingDisposeScope. However storing and re-opening a scope would require some changes to be supported.So it can even look something like this:
In this way there's no need to define a
Disposabletype, and no need to use aDisposableStack. Even theusingkeyword can be omitted. A function created disposable resource no longer has to return aDisposabletype. Every function just return whatever value they produce, anddeferstatement can be inserted at any location to dispose resource at the enclosingDisposeScope.In this way, a disposal cannot be scheduled with
deferif there's no enclosingDisposeScope. And the only way to register disposal operation is to usedefer, 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 todeferto an enclosingDisposeScope, resulting an error being thrown, or it's guarenteed to be disposed when the enclosingDisposeScopeclosed. If a resource is living longer than you expected, it's very likely being deferred to some long runningDisposeScope, and you can easily find both theDisposeScopeand the dispose method it referecnes in a heap dump. Diagnosing resource retaintion problems would be easier this way.Extending the lifetime of a
DisposeScopeResource 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
DisposeScopemade 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 theDisposeScopecan enclose. When that happen, we will have use-after-free problems:Note that
DisposableStackalso has this problem:However, if we decouple the
DisposeScopefrom 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:An immediate observation tell us, we can make it work if we add the following rule for a
DisposeScopedisposal: Only dispose aDisposeScopewhen all the references to that scope were gone (unreachable / garbage collected).We can manually analyze the bahavior by denoting reference counts of the 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
DisposeScopewe 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 anasync disposescope instead, even the deferred function is synchronous:We can write self managed resources:
We can even keep improving this by integrating async context tracking mechanism like Node.js
AsyncLocalStorage/AsyncResourceor theAsyncContextproposal. So theDisposeScopeitself is anAsyncResource.There are however some performance implications if we binds the lifetime of an async
DisposeScopeto 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.The text was updated successfully, but these errors were encountered: