Stream read hangs when stream is closed during a BYOB read

[bschick]

When constructing a ReadableStream and then reading it with a ReadableStreamBYOBReader (for example to control the size of reads), if the stream is closed during the read request without enqueued data, the read request hangs. This is problematic when the stream is used from a library by unknown callers. The solution is to not ignore BYOB in the stream implementation and call close() followed by controller.byobRequest.respond(0), but that is unexpected when the readable stream otherwise ignores BYOB. Why isn't calling close() alone sufficient?

More detail, code examples, and the workaround can be found in this Stack Overflow question: https://stackoverflow.com/questions/78804588/why-does-read-not-return-in-byob-mode-when-stream-is-closed

[MattiasBuelens]

Why isn't calling close() alone sufficient?

With the current ReadableByteStreamController API, we always need an explicit call to either respond() or respondWithNewView() in order to pass control of the BYOB view back to the BYOB reader. Otherwise, we don't really know when you are "done" using it.

For example, consider the following stream:

const stream = new ReadableStream({
  type: 'bytes',
  async pull(controller) {
    const { byobRequest } = controller;
    const { buffer, byteOffset, byteLength } = byobRequest;

    const newBuffer = buffer.transfer();
    const bytesWritten = await fillBuffer(buffer, byteOffset, byteLength);

    if (bytesWritten === 0) {
      controller.close();
      byobRequest.respondWithNewView(new Uint8Array(newBuffer, byteOffset, 0));
    } else {
      byobRequest.respondWithNewView(new Uint8Array(newBuffer, byteOffset, byteLength));
    }
  }
});

Here, the stream is transferring the BYOB buffer before filling it. It can do this with an explicit .transfer(), or by passing it in the transfer list of a Worker.postMessage() call. The result is that the original ArrayBuffer of the BYOB view is now detached, and in order to fulfill the BYOB request, the stream must pass the "new" ArrayBuffer back to the stream.

The ReadableStream can't do this by itself at the moment that you call close(). It can see that its ArrayBuffer is now detached, but it doesn't know where the "non-detached" version of that buffer is currently located. (Even if it could, trying to take ownership of that buffer could cause all sorts of problems when the buffer is being used by another thread.)

I agree that it's annoying that you have to call respond() even if you never transfer the BYOB buffer (which is the likely case for most byte streams). I'm wondering if we can improve this.

• Can we have close() automatically call respond(0) if the BYOB buffer is not detached?
• Or would that cause too many headaches if you "conditionally" transfer the BYOB buffer, and now you're debugging weird edge cases where sometimes the stream gets stuck (instead of the status quo, where it always gets stuck)?

[bschick]

Edited 8/16 to fix (at least one) misunderstanding on my part

I certainly have not considered all the potential scenarios, but I also find it odd that you can even respond() with an empty view after closing a stream (and doing so before closing the stream is an error). This is what I would expect:

• After calling close() you can no longer respond(), and doing so would be an error. Other than the scenario we're discussing, which seems like it shouldn't exist, a respond after close is likely a coding mistake
• Responding with zero length view should either never be an error or always be an error
• As you proposed, calling close() would create an empty view automatically. But consider doing that based on the lack of remaining data from previous responds rather than attached BYOB buffer. Then it is not conditional, and you never hang the reader upon close().
• Or make calling close() when the BYOB buffer is detached an error.
• Either way, it seems preferable for close() to be documented to mean the user code must be done with the buffer and that the stream implementation can take it back. Calling byobRequest.respond(0) is not explicitly passing the buffer back either... so why support indirect signaling of buffer release that way after calling close() rather than just having close() be the signal?

[bschick]

I was also thinking that the current design seems susceptible to race conditions unless the implementer ensures there is either never an await between a respond() and a close() or always a respond(0) after close(). For example, if a pull function does a respond(), then awaits on something, then calls close() you may end up with occasional BYOB reader hangs even though the code structure implies you did an explicit respond before the close. None of the docs I've seen call this out.

[bschick]

@MattiasBuelens Any feedback the last two comments?