Open littledan opened 2 years ago
I'd like to understand better the motivation for adding
@@asyncDispose
and AsyncDisposalStack now, in this proposal, as opposed to focusing on just launching disposal and not waiting on it. Apologies, I haven't caught up on all the threads where this has been discussed; feel free to point me to a past explanation if one is already written.
Launching and not waiting on an asynchronous dispose is extremely unsafe as it can introduce race conditions in asynchronous code. Asynchronous dispose is also necessary for scenarios like three-phase commit transactions, where you need to ensure that the transaction has successfully committed (and all changes flushed to their respective database/system) before continuing. You also would not be able to be properly aware of any exceptions raised, which is treated as a potential crash condition in NodeJS (unhandled rejections). I would strongly advise against a "fire and forget" approach for disposing resources asynchronously. We already have precedent for this as well with for await
, Symbol.asyncIterator
, and an async iterator's return
method, as for await
will Await the result of return
.
Your section on how web APIs would integrate with disposal is really interesting and provides good motivation, but not to the point where I fully understand the design. It's notable that these examples include a method
.close()
both in cases that returnundefined
and cases that return a Promise. I'm wondering, would this be a reasonable pattern to follow for@@dispose
, allowing it to return a Promise, rather than using a separate symbol?
The fact that the web APIs are inconsistent between synchronous and asynchronous cleanup in .close()
is one of the motivating cases for this proposal. Clearly defining the difference between synchronous and asynchronous dispose and having tools (using
, DisposableStack
, and AsyncDisposableStack
) would be invaluable to having a consistent mechanism for managing resources. I would be strongly against reusing @@dispose
for both sync and async dispose.
In this case, the only difference between
using x = y
andusing (await x = y) { }
would be whether we bother waiting on the value returned from@@dispose
--in the former case, we're fine to let disposal happen in the background, but the same thing executes. Similarly, for AsyncDisposableStack, the difference would be whether we block on starting one disposal for the next one, or just launch them all in parallel (and maybe this would be a an option to the constructor rather than a separate class).
It's almost never a good idea to let explicit disposal happen in the background, especially if such cleanup is critical. As mentioned above, resource disposal should be explicit to avoid race conditions in async code and to properly catch and handle exceptions during cleanup.
I'm asking this partly because I'm having trouble understanding when it is important to block control flow on the disposal completing, as opposed to being content having launched it off in the background. Presumably these use cases went into the design of the referenced web APIs, but I don't know where to find more information about that.
The entire premise of this proposal is to explicitly handle and observe cleanup and any exceptions that occur. I would not be comfortable with weakening this stance.
(A way to avoid coming to a conclusion on this question would be to just skip
@@asyncDispose
and AsyncDisposalStack for now and introduce them later, but given the pattern's usage in the web platform, I can sympathize with the impulse for adding it now.)
While tempting, I've found that async disposal comes up often enough that I'd rather have something in the MVP to support it. The only reason I'm comfortable with postponing using await
to a follow on is that the groundwork would be laid for async disposal through the introduction of @@asyncDispose
and AsyncDisposableStack
.
Yeah, transaction commit is an especially important use case; thanks for explaining. Do you see the web platform use cases as ones where things will get out of order? They look a bit more like resource cleanup and less likely to cause race conditions, but maybe I'm missing something.
I am wondering, how high do we all think the risk is of starting with synchronous-only disposal (e.g., C++, or maybe earlier states of evolution of C#)? I guess if the lack of async disposal is a big source of bugs, we should be able to see them in these sorts of cases. Is the risk so high that introducing syntax for one and not the other is something to be avoided?
Do you see the web platform use cases as ones where things will get out of order? They look a bit more like resource cleanup and less likely to cause race conditions, but maybe I'm missing something.
Resource cleanup can still result in race conditions. Even if that's not a concern, memory pressure can still be an issue, especially on embedded or other resource-limited environments. An app may want to ensure that an expensive resource its holding on to, such as an audio or video clip, has been fully removed from memory before loading another one.
I am wondering, how high do we all think the risk is of starting with synchronous-only disposal (e.g., C++, or maybe earlier states of evolution of C#)? I guess if the lack of async disposal is a big source of bugs, we should be able to see them in these sorts of cases. [...]
I think we will likely end up with two very different syntaxes to support sync and async disposal. I've come to believe that the RAII-style using x = ...
declarations are the best approach for sync disposal. However, the I also believe such a declaration is unlikely to address @erights's and others concerns about an unmarked implicit await
at the end of a block.
At the end of the day, I think we will probably end up with this:
// sync disposal
{
using x = ...;
...
} // dispose at end of block
// async disposal
using await (y = ...) {
...
} // dispose at end of block
I've considered reintroducing the original using
statement as a bridge between the two forms, but I'd still rather have the RAII-style to start.
[...] Is the risk so high that introducing syntax for one and not the other is something to be avoided?
I think both are extremely valuable, but I think that having at least AsyncDisposableStack
for async resources is at least a good interim solution, since it reduces try..finally
boilerplate and would still be compatible with a future async using
follow-on:
async function f() {
const stack = new AsyncDisposableStack();
try {
const ares1 = stack.use(new AsyncResource1());
const ares2 = stack.use(new AsyncResource2());
...
}
finally {
await astack.disposeAsync(); // disposes of ares2 and ares1, but unfortunately suppresses errors from the `try`.
}
}
If a primary motivator for including @@asyncDispose
as part of this initial proposal is for its expected use helping to unify the web-APIs, I wonder if there has been any single from web-folk that they would indeed adopt this? If so that may really help solidify the motivation for it not being a separate follow on proposal.
If a primary motivator for including
@@asyncDispose
as part of this initial proposal is for its expected use helping to unify the web-APIs, I wonder if there has been any single from web-folk that they would indeed adopt this? If so that may really help solidify the motivation for it not being a separate follow on proposal.
As a classical bystander I would say I am strongly against having the async disposal being separated into its own proposal since it significantly footguns the usage of disposables since there are a lot of stuff that would only be usable or feasible to do so if the disposal will be async like file handle close, database transactions, multi-threading with locks, web requests, etc.
I'm kind of unclear why the web platform (or Node.js) would adopt @@disposeAsync
if there's no syntax support for it? Just adding a bunch of aliases/wrappers doesn't really buy much. (But I might not be understanding the proposal; there are a lot of moving parts.)
@domenic Syntactic support for async using
is forthcoming, but has been postponed temporarily due to it potentially needing to leverage async do
expressions (see tc39/proposal-async-explicit-resource-management#1 for additional context). The proposal should be fairly stable at this point. I don't foresee any major changes to the sync version, and the only change to the async version may be the spelling of the asynchronous block syntax necessary to meet @erights concerns about avoid implicit await
points in the language.
Also, I've opened https://github.com/whatwg/html/issues/8557 for discussion about potential WHATWG API integration for either the sync or async APIs.
I'd like to understand better the motivation for adding
@@asyncDispose
and AsyncDisposalStack now, in this proposal, as opposed to focusing on just launching disposal and not waiting on it. Apologies, I haven't caught up on all the threads where this has been discussed; feel free to point me to a past explanation if one is already written.Your section on how web APIs would integrate with disposal is really interesting and provides good motivation, but not to the point where I fully understand the design. It's notable that these examples include a method
.close()
both in cases that returnundefined
and cases that return a Promise. I'm wondering, would this be a reasonable pattern to follow for@@dispose
, allowing it to return a Promise, rather than using a separate symbol?In this case, the only difference between
using x = y
andusing (await x = y) { }
would be whether we bother waiting on the value returned from@@dispose
--in the former case, we're fine to let disposal happen in the background, but the same thing executes. Similarly, for AsyncDisposableStack, the difference would be whether we block on starting one disposal for the next one, or just launch them all in parallel (and maybe this would be a an option to the constructor rather than a separate class).I'm asking this partly because I'm having trouble understanding when it is important to block control flow on the disposal completing, as opposed to being content having launched it off in the background. Presumably these use cases went into the design of the referenced web APIs, but I don't know where to find more information about that.
(A way to avoid coming to a conclusion on this question would be to just skip
@@asyncDispose
and AsyncDisposalStack for now and introduce them later, but given the pattern's usage in the web platform, I can sympathize with the impulse for adding it now.)