Open bschick opened 2 months ago
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.
close()
automatically call respond(0)
if the BYOB buffer is not detached?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:
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 mistakeclose()
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().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?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.
@MattiasBuelens Any feedback the last two comments?
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 bycontroller.byobRequest.respond(0)
, but that is unexpected when the readable stream otherwise ignores BYOB. Why isn't callingclose()
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