Is this proposal strict enough for its use cases?

[andreubotella]

This proposal's name is "strict enforcement of using", but while it solves the footgun of forgetting using, it is not strict against unintended usage of the disposable API.

You might expect that a strict enforcement of an RAII-like pattern would guarantee not only that the [Symbol.dispose] method is always called, but that it is called before the current function returns, and that the ordering of [Symbol.enter] and [Symbol.dispose] calls across multiple disposables is properly stacked. Since this proposal allows JS code to call the [Symbol.enter]() and [Symbol.dispose]() methods, it guarantees none of these properties.

As an example use case where these properties matter, in the AsyncContext proposal we were discussing the possibility of a disposable API (see tc39/proposal-async-context#60 and nodejs/node#52065), that would switch the current context when entered and restore the previous one when disposed. However, since the AsyncContext machinery is unaware of function boundaries, the possibility of an entered but non-disposed scope would pollute the caller's context:

import { callee } from "some-library";

const asyncVar = new AsyncContext.Variable();

function caller() {
  asyncVar.run(42, () => {
    console.log(asyncVar.get());  // 42

    callee();

    // As the AsyncContext proposal currently stands, there would be nothing
    // callee can do that can make asyncVar.get() not return 42. However...
    console.log(asyncVar.get());  // undefined
  });
}

// some-library

// The AsyncContext snapshot at the time this module was evaluated, which is
// likely empty.
const initialSnapshot = new AsyncContext.Snapshot();

export function callee() {
  const scope = new AsyncContextScope(initialSnapshot);
  scope[Symbol.enter]();
}

Now, it is possible that this is a one-off, or a special case, and that most disposables would benefit from fixing the footgun without needing this level of strictness. But that would need to be looked into.

[ggoodman]

At first I was trying to think through some approach wherein some clever indirection could add some stronger guarantees to these Resources. I was thinking, "what if Symbol.enter() received an argument from the runtime with which the Resource author can register the instance?" But then I realized that this would just add another hoop to jump through without actually preventing the foot-gun.

I couldn't think of many precedents in terms of APIs that strictly required runtime support like this. The obvious one that did come to mind though, was Promise and the await keyword. I think this is a relevant precedent in that the API was designed in a way that had built-in foot guns like: await new Promise(() => {});. The success of Promise and async / await despite these weaknesses suggests that perhaps it's reasonable to compromise when it allows for user-land shims and transpilation solutions to exist.

[rbuckton]

it is not strict against unintended usage of the disposable API.

Our position is that if you are explicitly calling [Symbol.enter](), you are intentionally opting in to doing everything yourself. It is imperative that there is a manual way to interact with disposables if you want to build your own resource management building blocks on top of disposables. [Symbol.enter]() is the way you do that.

You might expect that a strict enforcement of an RAII-like pattern would guarantee not only that the [Symbol.dispose] method is always called, but that it is called before the current function returns, and that the ordering of [Symbol.enter] and [Symbol.dispose] calls across multiple disposables is properly stacked. Since this proposal allows JS code to call the Symbol.enter and Symbol.dispose methods, it guarantees none of these properties.

That is explicitly not a feature of this proposal and I am not making any such guarantees. Symbol.enter is intended purely as a stumbling block to guide people to using. I do not intend it to have any other power or capability over and above that. If you explicitly call [Symbol.enter](), you are expected to know what you are doing. If that means renaming the symbol to Symbol.enter_DO_NOT_USE_OR_YOU_WILL_BE_FIRED or something similar but less meme-y to get the point across, I'm perfectly fine with that.

However, since the AsyncContext machinery is unaware of function boundaries, the possibility of an entered but non-disposed scope would pollute the caller's context

Sure, but if you've chosen to explicitly call [Symbol.enter]() without manually handling cleanup, then you've written buggy code.

[andreubotella]

That is explicitly not a feature of this proposal and I am not making any such guarantees. Symbol.enter is intended purely as a stumbling block to guide people to using. I do not intend it to have any other power or capability over and above that. If you explicitly call [Symbol.enter](), you are expected to know what you are doing. If that means renaming the symbol to Symbol.enter_DO_NOT_USE_OR_YOU_WILL_BE_FIRED or something similar but less meme-y to get the point across, I'm perfectly fine with that.

I think the name [Symbol.enter] might be fine, but "strict enforcement" as the name of the proposal might suggest the wrong thing.

[jridgewell]

Our position is that if you are explicitly calling [Symbol.enter](), you are intentionally opting in to doing everything yourself. It is imperative that there is a manual way to interact with disposables if you want to build your own resource management building blocks on top of disposables. [Symbol.enter]() is the way you do that.

Not arguing against that, but we are saying that it's not a strict enough guarantee to solve AsyncContext's problem. We'd like to explore a way to do both, so that other APIs that care about strict enforcement are able to implement it.

Sure, but if you've chosen to explicitly call [Symbol.enter]() without manually handling cleanup, then you've written buggy code.

It's not just manual calls to scope[Symbol.enter]() that are a problem. Even DisposableStack if you were to properly register it still allows an accidental leak that we'd like to prevent.

class Foo {
  private stack = new DisposableStack();

  example() {
    const scope = new AsyncContext.Scope(someSnapshot);
    disposable.use(scope);

    // Do stuff…

    // didn't dispose here!
  }

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


const v = new AsyncContext.Variable();
using f = new Foo();

v.run(1 , () => {
  assert.equal(v.get(), 1);

  // f is properly using a DispoableStack, but that's not enough
  // to prevent a leak.
  f.example();

  // f's context has leaked out of it's method call.
  assert.equal(v.get(), undefined);
});

Ideally, we'd like a way to detect syntatic using, because that's the only thing that guarantees our ability to restore the async context state to what it was prior to the mutation.

[shicks]

@jridgewell

Ideally, we'd like a way to detect syntatic using, because that's the only thing that guarantees our ability to restore the async context state to what it was prior to the mutation.

This seems problematic w.r.t. syntax transformation. This sort of feature would (by definition) be untranspilable, making it useful (at best) only in the very-narrow case of transpiling later language features that would depend on it.

[ljharb]

Such a mechanism would have to also support eg const s = new DisposableStack(); s.use(f); using s;, and I'm not sure how that would be possible in a reliable fashion.

[jridgewell]

@shicks

This seems problematic w.r.t. syntax transformation. This sort of feature would (by definition) be untranspilable, making it useful (at best) only in the very-narrow case of transpiling later language features that would depend on it.

Someone mentioned a function.using meta property in Matrix, and that would work fine and be polyfillable with minimal transform. Just inect a global, private boolean, set it to true immediately before using, and set it back to false immediately after.

---

@ljharb

Such a mechanism would have to also support eg const s = new DisposableStack(); s.use(f); using s;, and I'm not sure how that would be possible in a reliable fashion.

During the using s, the DisposableStack[Symbol.enter] method would be able to detect it's in a syntatic using. But the original f cannot, because it's already been entered by the s.use(f) call.

[ljharb]

Right - which is why "enforcing syntactic usage" won't work, because that would give you a false negative.

[mhofman]

Someone mentioned a function.using meta property in Matrix, and that would work fine and be polyfillable with minimal transform. Just inect a global, private boolean, set it to true immediately before using, and set it back to false immediately after.

Not quite. It would have to be reset for further nested stacks.

[jridgewell]

@ljharb

Right - which is why "enforcing syntactic usage" won't work, because that would give you a false negative.

Not from my point of view. I specifically do not want it to pass, so it's giving a true negative. It is not acceptable to use AsyncContext within a DisposableStack.

---

@mhofman

It would have to be reset for further nested stacks.

Ah, yes, it should reset to the previous value. But still easily implementable with a transform.

[ljharb]

Why isn't that acceptable? It's going to be properly disposed.

This really feels like it should just be a linter rule, like no-floating-promises - especially given that there's differing opinions about what level of enforcement is desired.

[rbuckton]

@jridgewell and I have been discussing this on Matrix. The kinds of guarantees that AsyncContext.Scope needs would preclude it from being used in a DisposableStack and would prohibit any kind of composition. I'm concerned that a carve out for this case could poison adoption in the ecosystem as users will have less confidence about whether a resource can be used with using or DisposableStack interchangeably, or whether refactoring from

using x = new Resource();

to

const getResource = () => new Resource();
using x = getResource();

will break their code.

The intended use of AsyncContext.Scope does overlap with using, so it makes sense that they would seek to employ using for that purpose. However, given the potential impact on adoption as a result of user confusion, I'd prefer if the more specific needs of AsyncContext.Scope be handled by way of a syntactic opt-in as it would give users more confidence in making a determination as to whether simple refactors will be safe, e.g., using scope(ctx), or something to that effect.

I have also proposed an alternative approach, where each successive new Scope is maintained in a stack, such that when AsyncContext.run exits it would clean up any non-exited Scope objects and then throw. This would provide feedback to the developer about an incorrect use of the feature without necessitating a special case in using, but that did not seem to sufficiently address their concerns.