Skip to content
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

Make disposables composable - Syntax for returning the cleanup safely #55

Closed
acutmore opened this issue Jan 1, 2021 · 8 comments · Fixed by #56
Closed

Make disposables composable - Syntax for returning the cleanup safely #55

acutmore opened this issue Jan 1, 2021 · 8 comments · Fixed by #56

Comments

@acutmore
Copy link

acutmore commented Jan 1, 2021

TLDR:

The proposal in this repo helps the final 'end of the chain' but it doesn't help the intermediary functions that are creating the resources on behalf of the caller. The resource creation and accompanying cleanup may need to be threaded through multiple layers and still be exception safe.

Background:

A pattern found in many libraries is for resource cleanup to be returned as a function:

// RxJs
let interval$ = new Observable(observer => {
  let i = 0;
  let timerId = setInterval(() => observer.next(i++), 1000);
  
  // return cleanup
  return () => {
    clearInterval(timerId);
  }; 
});

// React.js
useEffect(() => {
  let timerId = setInterval(() => {
    setState(i => i + 1);
  }, 1000);
  
  // return cleanup
  return () => {
    clearInterval(timerId);
  };
})

// Svelte
let intervalStore = readable(0, function start(set) {
  let i = 0
  const timerId = setInterval(() => {
    set(i++);
  }, 1000);

  // return cleanup
  return () => {
    clearInterval(timerId);
  };
});

To make use of the using syntax a similar pattern is required with the difference being that the cleanup function is found by accessing the Symbol.dispose property on the returned object.

Problem:

Returning the clean-up function at the end like this has similar issues as the calling side has; exceptions stop the cleanup.

Calling side issue that using addresses:

const file = openFile();
... // code that could throw
file.close(); // oops not in a finally block

Exception throwing issue on callee side:

function openMultipleResources() {
    return [openResource(1), openResource(2)] // opening on behalf of caller so can't use 'using'
}

// end of chain - can now use 'using' but if 'openResource(2)' throws then resource-1 won't be cleaned up.
try using (const [r1, r2] = openMultipleResources()) {
  ...
}

Current Solution:

To safely create the resource to pass to the caller the code needs to be re-written using try/catch as below. But this type of code seems to be the type of code this proposal is trying to help code authors move away from.

function openMultipleResources() {
   let r1;
   try {
     r1 = openResource(1);
     return [r1, openResource(2)];
   } catch (err) {
      if (r1) {
         r1[Symbol.dispose]();
      }
      throw err;
   }
}

Ideation:

Could a function declare syntactically that it creates resources that it intends the caller to dispose of, this way the language itself could ensure the resources are closed even if an exception is thrown mid way through. It could also help IDEs suggest when to use using.

// 'resource' keyword bike-shed
function openMultipleResources() {
  return [resource openResource(1), resource openResource(2)];
}

// or 'usable' ???
function openMultipleResources() {
  return [usable openResource(1), usable openResource(2)];
}

being roughly equivalent to:

function openMultipleResources() {
  const _disposables = [];
  try {
    const _r1 = openResource(1);
    const _d1 = _r1[Symbol.dispose];
    if (typeof d1 !== 'function') {
      throw new TypeError();
    }
    _disposables.push(_d1);
    
    const _r2 = openResource(2);
    const _d2 = _r2[Symbol.dispose];
    if (typeof d2 !== 'function') {
      throw new TypeError();
    }
    _disposables.push(_d2);

    return [_r1, _r2];
  } catch(err) {
     for (const r of _disposables.reverse()) {
       r(); // should probably try/catch here too and re-throw AggregateError
     }
     throw err;
  }
}

For the libraries where the only expected return value is the cleanup, perhaps a resource function and resource () => would automatically return all the created resources in an aggregated Disposable (#41).

/* new modern seInterval util, could be part of node.js timers package for example */
function modernInterval(callback, ms) {
  let id = setInterval(callback, ms);
  return {
    [Symbol.dispose]() { clearInterval(id); }
  };
}

// RxJs
let interval$ = new Observable(resource (observer) => {
  let i = 0;
  resource modernInterval(() => observer.next(i++), 1000);
  
  observer.next(otherValue()); // if  otherValue() throws timer is still disposed

  // implicit return of Disposable containing above resource  
});

// React.js
useEffect(resource () => {
  resource modernInterval(() => {
    setSeconds(i => (i + 1) % 60);
  }, 1_000);

 resource modernInterval(() => {
    setMinutes(i => (i + 1) % 60);
  }, 60_000);
  
  // implicit return of Disposable containing both intervals  
});

Trying to return any other value would be a syntax error.

new Observable(resource (observer) => {
  let i = 0;
  resource modernInterval(() => observer.next(I++), 1000);
  
  return; // Allow bare return to allow for early logical returns
  return someValue; // SyntaxError - attempting to return a different value is not allowed in a 'resource' function
});

Thoughts?

@fstirlitz
Copy link

Introduce a move operator.

function openMultipleResources() {
	using const res1 = openResource(1);
	/* A */
	using const res2 = openResource(2);
	/* B */
	const result = [>>>res1, >>>res2];
	/* C */
	return result;
}

Semantics: When NAME is a lexically-managed binding, an expression of the form >>> NAME evaluates to the contents of NAME, and has the side effect that disposal through that binding is suppressed. So in the above, if an exception occurs at point A, res1 will be disposed. If it occurs at point B, both res1 and res2 will be disposed. But if one occurs at point C, both will leak.

For transferring resource ownership into closures, maybe something like:

() using x, >>>y, z => f(x, y, z)

which would then be equivalent to

((x, y, z) => {
	const closure = () => f(x, y, z);
	
	closure[Symbol.dispose] = () => {
		using const _x = x;
		/* y skipped */
		using const _z = z;
	};
	
	return closure;
})(>>>x, >>>y, >>>z)

And then disposing should be recursive, so that this works:

function makeObjectWithStuff() {
	using const res1 = openResource(1);
	using const res2 = openResource(2);

	/* if an exception is thrown here, both are disposed */

	return {
		/* if execution reaches the evaluation of the object constructor,
		 * ownership is transferred to the constructed object
		 */
		foo() {
			use(res1, res2)
		},

		[Symbol.dispose]: () using res1, res2 => {}
	}
}

@acutmore
Copy link
Author

acutmore commented Sep 21, 2021

Hi @rbuckton ,

the linked PR that closes this. Seems like the “issue” would still apply to the new design? Even with the Disposable.fromaddition (which I like).

Thinking about this issue again with 9 months of hindsight. I think what I was trying to get to was having the same mechanism but for catch, to compliment thefinally semantics.

For example:

using const x = 
holding const y = 

gives:

let _x, _y;
try {
  const x = ;
  _x = __getDispose(x);
  const y = ;
  _y = __getDispose(y);
} catch(e) {
  __dispose(_y);
  throw e;
} finally {
  __dispose(_x);
}

When we are “using” something then we are the end of the chain, and any completion should clean up the resource.
When we are “holding” something then we are a middleware, creating a resource on behalf of the caller, this resource will be returned and be the responsibility of the caller. However if an exception is raised before responsibility is passed over then we do want the resource to be cleaned up.

This could be a follow on proposal. As it seems purely additive.

@rbuckton
Copy link
Collaborator

rbuckton commented Sep 21, 2021

For Disposable, one option could be to add .add/.delete to the API, so that for your holding example, you could write:

{
  using const x = ...;
  const y = ...; // thing you want to hold
  using const holder = Disposable.from(y);

  // do work that could throw (a)

  // stop tracking `y` in the holder
  holder.delete(y);
  return y;
  
  // if (a) throws, `holder`, `y`, and `x` are disposed
  // if (a) does not throw, `holder` and `x` are disposed, but `y` is not because it was removed.
}

I'm not sure I want to extend the Disposable API in that fashion however, as it doesn't work well with the new Disposable(callback) constructor.

Another option could be a static method on Disposable akin to Proxy.revocable:

{
  using const x = ...;
  const y = ...;
  using const holder = Disposable.hold(y);

  // do work that could throw
  
  holder.release(); // unenlists `y` from the holder. It will no longer be disposed at the end of the block.
  return y;
}

In absence of such an API, you could still be accomplish this in userland:

{
  using const x = ...;
  const y = ...;

  // holder to track safe completion/cleanup of 'y'
  let ok = false;
  using const (new Disposable(() => {
   if (ok) return;
   using const (y);
  }));

  // do work that could throw

  ok = true;
  return y;
}

@acutmore
Copy link
Author

“Revocable” disposables also came to my mind too.

consider me convinced that this can be solved effectively in user land. Thanks @rbuckton!

@rbuckton
Copy link
Collaborator

I think this is something we can consider in a future/add-on proposal as it is outside the scope of the base functionality, but something like hold could be a simple addition to the API.

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 8, 2021

Please see #80 where I've proposed introducing a DisposableStack/AsyncDisposableStack which models some of the concepts discussed here.

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 8, 2021

For example:

// "revoking" a dispose
using const holder = new DisposableStack();
const y = holder.use(...);
...
// stop tracking y
holder.move();
return y;

// opening multiple resources
function openMultipleResources() {
  using const stack = new DisposableStack();
  const res1 = stack.use(openResource(1));
  const res2 = stack.use(openResource(2));

  // if we've reached this point, both resources were successfully
  // allocated. Move them off the stack so that they aren't
  // disposed when the block exits.
  stack.move();
  return [res1, res2];
}

@acutmore
Copy link
Author

acutmore commented Nov 8, 2021

Thanks for the update. That looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants