proposal-explicit-resource-management icon indicating copy to clipboard operation
proposal-explicit-resource-management copied to clipboard

Masking disposable

Open jasnell opened this issue 2 years ago • 16 comments

@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:

{
  using foo = bar();
  {
    using baz = foo;
  }
  console.log(foo);
}

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...?

// library.js
export function handle(xyz) {
  using abc = xyz;
}

// main.js
import { handle } from 'library.js'

{
  using foo = bar();
  handle(foo);
  console.log(foo);
}

Again, as I understand it, when the scope for handle(...) exits the Symbol.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.

handle(undisposable(foo)); // throws! because `undisposable(foo)` returns an object that proxies `foo` without `Symbol.dispose`.

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?

jasnell avatar Sep 27 '23 23:09 jasnell

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.

ljharb avatar Sep 27 '23 23:09 ljharb

hmm, ok, that constrains a few options I think but it makes sense. Thank you!

jasnell avatar Sep 27 '23 23:09 jasnell

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.

mhofman avatar Sep 27 '23 23:09 mhofman

You may also wish to take a look at https://github.com/tc39/proposal-explicit-resource-management/issues/195, which suggests a mechanism by which the capability to close a resource could be separated from the resource itself.

bakkot avatar Sep 27 '23 23:09 bakkot

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 Symbol.dispose is called without it being... surprising. The short answer is yes, it's all manageable but as you say, we still have to think about what scope owns the lifetime. That doesn't change.

I do think there are some potential footguns buried in here but some well crafted linting rules ought to be able to catch those.

jasnell avatar Sep 27 '23 23:09 jasnell

Another quick clarification, I assume the following is also true...

function foo() {
  using xyz = bar();
  return promise.then(() => {
    // xyx captured by closure will have been disposed by this point!
  });
}

jasnell avatar Sep 27 '23 23:09 jasnell

I assume the following is also true...

Correct.

bakkot avatar Sep 27 '23 23:09 bakkot

If your intent is to explicitly give ownership of a resource to another function, one option is to emulate DisposableStack.prototype.move(), in which you create a copy of the object and give it the original object's internal state, then clear out the original object's internal state w/o disposing.

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);
}

rbuckton avatar Sep 28 '23 00:09 rbuckton

function foo() {
  const xyz = bar();
  return promise.then(() => {
    return xyz;
  },(e) => {
    xyz[Symbol.dispose]();
    return Promise.reject(e);
  });
}

@jasnell Shouldn't we leave using to the caller as above...?

juner avatar Sep 28 '23 00:09 juner

Another quick clarification, I assume the following is also true...

Yes. using lifetime is scoped to the block. If you wish it to live long enough to be reachable in Promise.prototype.then you have two options:

  1. Switch to async/await:
async function foo() {
  using xyz = bar();
  const result = await promise;
  // xyx has not been disposed.
  return result;
}
  1. Manually handle lifetime, or leverage DisposableStack.prototype.move
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 DisposableStack is to give you additional control over lifetime for situations like these, without having to revert back to manual cleanup behavior using try..finally.

rbuckton avatar Sep 28 '23 00:09 rbuckton

@juner:

Shouldn't we leave using to the caller as above...?

Generally, yes, but ... Misbehaving Code :-)

jasnell avatar Sep 28 '23 15:09 jasnell

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.

jasnell avatar Sep 28 '23 15:09 jasnell

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 delete obj[Symbol.dispose] / obj[Symbol.dispose] = null inside of a dispose call?

guybedford avatar Nov 15 '23 22:11 guybedford

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 using captures [Symbol.dispose] during initialization.

Generally, it's better to track whether dispose has been called using something like a private field.

rbuckton avatar Nov 15 '23 22:11 rbuckton

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 object[Symbol.dispose] = () => {}; might not be so bad for dead objects.

guybedford avatar Nov 15 '23 22:11 guybedford

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 FooContainer example in https://github.com/tc39/proposal-explicit-resource-management/issues/204#issuecomment-1738277085, when the disposer sets this.#foo = undefined.

rbuckton avatar Nov 15 '23 22:11 rbuckton