Discussion: How should streams handle Explicit Resource Management

[jasnell]

What is the issue with the Streams Standard?

The Explicit Resource Management proposal (https://github.com/tc39/proposal-explicit-resource-management) is moving forward in TC-39 and implementers are making progress. One thing I'm not sure has been discussed here is how, if at all, the streams spec should implement ERM.

Let's consider ReadableStream first. Let's assume that it is decided that ReadableStream should be disposable...

Would it support Symbol.dispose (sync disposal) AND Symbol.asyncDispose or just async dispose? Let's use an example

const rs = new ReadableStream({
  async cancel() {
    // just fake some async activity
    await scheduler.wait(1000);
  }
});

{ using stream = rs }

Would we expect that the sync disposer triggers the async cancel algorithm if the ReadableStream is still open and readable? Does a sync disposer even make sense in this case or should it always be await using?

Does it even make sense to support disposing of the ReadableStream like this? If it is, would it just simply be the equivalent of calling cancel() with no reason?

Let's consider a reader:

{
  using reader = rs.getReader();
}

When the block exits, do we release the lock on the stream or do we cancel the stream via the reader? Releasing the lock is synchronous (suggesting that the sync Symbol.dispose would be sufficient) but canceling is async (suggesting that we would need Symbol.asyncDispose).

Let's consider a more complex case (this part may be more appropriate for the fetch repo so if we need to discuss it there, let me know)... Let's assume that the stream is attached to a disposable type, and let's assume that Response is defined as disposable... what should we expect would happen to the stream in the following case:

let stream;
{
  using resp = await fetch('http://example.com');
  stream = resp.body;
}
// Is the stream still readable here? Or was it disposed of?
const reader = stream.getReader();  // ??

The same basic questions apply to WritableStream, of course

const ws = new WritableStream({
  async abort() {
    await scheduler.wait(1000);
  }
});

let writer;
{
  // is sync dispose ok here? will it trigger the abort alg?
  using stream = ws;
  writer = stream.getWriter();
}
// is writer still usable?

[domenic]

I had always assumed ReadableStream would not be disposable, but readers would be, and disposing them would be equivalent to releaseLock().

[jasnell]

I definitely think that's the easiest approach to take and definitely has the fewest number of potholes ;-) ... and I'm 100% fine if we go with that, I just want to make sure the options are discussed rather than just assumed :-)

At least in theory, supporting async disposal of a readable stream could be the equivalent to cancel...

{
  await using rs = new ReadableStream({ ... });
}

By example, Node.js has implemented this pattern for Node.js streams, e.g.

// a node.js stream, not a ReadableStream
const r = new stream.Readable;
r[Symbol.asyncDispose]();   // cancels and destroys the stream

I can see some merit in the approach but I also like the simplicity of just saying, these streams aren't disposable, only the readers/writers are.

[annevk]

cc @MattiasBuelens @ricea @youennf

[annevk]

It would help if you could link to the proposal in OP here and in #1741.

[MattiasBuelens]

I agree that readers and writers should be sync-disposable. Conceptually, the "resource" they're holding is the lock on the stream, so when you dispose them, they should release that resource through releaseLock(). This also shows up in other languages, e.g. a Rust MutexGuard releases its lock on the parent Mutex when dropped.

Personally, I would expect readable and writable streams to be async-disposable.

• Async-disposing a ReadableStream should await readable.cancel().
• Async-disposing a WritableStream should await writable.close(). This aligns closer to the proposal for Node.js Writable to use writable.end().

This does mean that anything you do with that stream needs to be inside the block with the await using:

• If you acquire a reader/writer, you should release it within that block. This should be easy with using reader = readable.getReader().
• If you async-iterate the stream, you should either await using iterator = readable.values() so the iterator will properly release its lock when done, or just use for await (const chunk of readable) to do that automatically.

Piping is really tricky though. You're almost required to await every pipeTo(), otherwise the stream will still be locked at the end of the await using block. I don't even know what we'd do with pipeThrough()... 😕

Oh, and then there's tee() which keeps the original stream locked forever, so you can never dispose it... 😅

[jasnell]

Le's focus specifically on ReadableStream ... and just go through some of the scenarios to tease out the details (much of the following is obvious, I'm detailing it to make the conversation more concrete)

If the ReadableStream is not locked, then Async-disposal being the equivalent of await readable.cancel() makes sense... but o course, if the ReadableStream is locked, readable.cancel() fails. So the following would necessarily fail because the reader is not defined as being disposed.

{
  await using readable = new ReadableStream();
  const reader = readable.getReader();

  // Async dispose of readable fails since it is locked
}

If we define reader as being sync disposable such that it releases the lock, then the following should work just fine, releasing the lock then async canceling the stream...

{
  await using rs = new ReadableStream();
  using reader = rs.getReader();

  // equivalent to
  //    reader.releaseLock()
  //    await rs.cancel();
}

If the stream is locked to a reader, then I do think it is worth considering whether async using reader would be equivalent to await reader.cancel():

{
  const rs = new ReadableStream();
  await using reader = rs.getReader();

  // equivalent to
  await reader.cancel();
}

This would make sense especially given the idea that the Reader is essentially viewed as taking ownership over the stream. But this can be a bit tricky, of course.

In the pipeTo/pipeThrough scenario, the readable stream remains locked so await readable.cancel() would be expected to fail, so the following case would end with an error if async using readable assumes await readable.cancel()

let dest;
{
  await using readable = new ReadableStream();
  dest = readable.pipeThrough(new MyTransformStream());

  // async dispose fails because readable is locked by the pipe
}

But what if the destination is disposable? This should work fine, I think? The dest should be disposed of first, which should end up releasing the lock on readable, allowing it to be canceled?

{
  await using readable = new ReadableStream();
  await using dest = readable.pipeThrough(new MyTransformStream());
}

For tee, yeah... that's a bit wonky. Not sure what to do here.

{
  await using readable = new ReadableStream();
  const [b1, b2] = readable.tee();

  // Async dispose fails because readable is locked
}

{
  await using readable = new ReadableStream();
  await using [b1, b2] = readable.tee();

  // Async dispose of the branches works fine... but
  // Async dispose of readable still fails since canceling the branches does not release the lock on readable
}

Perhaps if we redefined it so that if all branches are canceled/closed the lock on the original stream is released?

Another interesting question, which may actually be a question for the TC-39 proposal authors because I'm not sure if this would even be possible... If the block exits because an exception is thrown, should the pending exception be passed into the readable.cancel(...) call when disposing the stream? I don't think this is currently possible with the current spec tho...

That is, for instance:

{
  async using readable = new ReadableStream();
  throw new Error('boom');

  // Should the async dispose be equivalent to `await readable.cancel(error)` where `error` is the error that is thrown
}

[domenic]

The discussion so far has convinced me that we should not make ReadableStream disposable. It adds to much complexity for not enough benefit, and encourages bad interactions with tee and piping. And, IMO it's bad practice to call cancel() with no argument anyway, and the using syntax does not give a place to insert the appropriate argument.

[jasnell]

Heh, I think I managed to convince myself of the same result :-) ... Having readers and writers be disposable so that they release the lock is fine, I think, but even that is probably not critical at all.

[jasnell]

Ok, I think given the discussion and after running through a ton of scenarios in which I've failed to convince myself that it could work, I think we've hit the right conclusion here and will close this issue. I'd say that even making readers and writers disposable is probably not something that is critical at all. Thank you for the consideration on it. I do think we need to have similar conversations and considerations for other web platform apis like Request, Response, etc, but those are best had in those repos.