Closed bakkot closed 2 years ago
I believe the problem is what to do if an error occurs during iteration. The elements that have been iterated on should be disposed of. For AsyncDisposableStack
that can only happen asynchronously, which means the "construction" operation needs to be awaited.
I do like the suggestion of the .of
helper as that translates to an asyncOf
for the async case.
Hm, even with of
you run into the problem that evaluating a later argument might throw, leaving earlier ones one un-disposed. And there's no chance to handle that, since the result of evaluating the first argument disappears into the void. I don't see a way around that problem (except a chainable builder, which probably isn't worth doing).
The of
helper could guarantee that if it fails iteration, it will dispose of the element that have been iterated so far (aka call dispose()
on itself).
Failing iteration isn't the only problem: consider
function getLock(name) {
if (!locks.has(name)) throw new Error(`${name} is not a known lock`);
return locks.get(name).lock();
}
using stack = DisposableStack.of(getLock('one'), getLock('two'));
What happens if two
is not a known lock? The result of getLock('one')
vanishes, never to be disposed.
Correct, and that's probably the difference between passing an iterable and passing multiple args or an array. If the iterable lazily allocates the resources, you're safe. Probably enough of a footgun to not provide such helpers.
It would be possible to do this with an executor callback that is expected to return an iterator (not iterable!) - in other words, make generator functions work:
{
using stack = new DisposableStack(function*() {
yield getLock1();
yield getLock2();
});
doOtherWork();
transfer(stack.move());
}
However, that syntax does not look much nicer than stack.use(…)
, and is one line longer. It's also trivial to implement yourself:
function disposableStackFrom(executor) {
using stack = new DisposableStack();
for (const res of executor()) stack.use(res);
return stack.move();
}
DisposableStack
went through a number of iterations. The earliest version had a .from(iterable)
static method, but it was found that this was error prone for the exact reasons mentioned above. It's much more reliable to explicitly call .use
for each item you create to ensure they are tracked correctly.
Yeah it's footgun, but if we don't have convenient constructor, I'm afraid that people will invent the similar helpers and create footguns eventually.
Yeah it's footgun, but if we don't have convenient constructor, I'm afraid that people will invent the similar helpers and create footguns eventually.
I'd rather the standard library not be the place to introduce such a footgun.
If I'm reading it correctly, there's currently no way to create a DisposableStack which owns certain resources in a single step - instead, you have to make a stack and then
.use
things.That seems kind of awkward for a lot of common cases. For example, imagine you're acquiring two locks, then performing some other work, then transferring ownership of those locks to someone else (if the other work succeeds):
This is exacerbated because the
use
method returns its argument, rather than (as forSet.prototype.add
) its receiver, so it can't be chained. (That particular decision seems fine, I'm just noting it.)It seems like it would be nicer to allow the creation and initialization to happen in a single step, for example by passing the resources as an iterable argument to the constructor, as for Set or Map:
Alternatively, say if we think the constructor arguments should be reserved for other uses in the future, we could add a helper like Array.of:
This is also nicer for the "DisposableStack as wrapper for function" use case.