Re: [whatwg/streams] Support teeing readable byte streams (#1114)

@MattiasBuelens commented on this pull request.



> +          if (forBranch2 === true) {
+            if (canceled1 === false) {
+              const clonedChunk = CloneArrayBufferView(chunk);
+              ReadableByteStreamControllerEnqueue(branch1._controller, clonedChunk);
+            }
+            if (canceled2 === true) {
+              chunk = new Uint8Array(chunk.buffer, chunk.byteOffset, 0);
+            }
+            ReadableByteStreamControllerRespondWithNewView(branch2._controller, chunk);
+          } else {
+            if (canceled2 === false) {
+              const clonedChunk = CloneArrayBufferView(chunk);
+              ReadableByteStreamControllerEnqueue(branch2._controller, clonedChunk);
+            }
+            if (canceled1 === true) {
+              chunk = new Uint8Array(chunk.buffer, chunk.byteOffset, 0);

> This seems like a significant footgun, especially given that none of the [example byte sources in the spec](https://streams.spec.whatwg.org/#example-rbs-push) do so.

Yes, it really is. We'll need to update those to handle it properly.

> Can we make this easier somehow?
> * Automatically introspect `byobRequest.view` and if it's transferred, call (the equivalent of) `respondWithNewView(constructedView)` ...

I don't think that's possible? If a buffer is transferred, we have no way to know *where it was transferred to*. We definitely cannot re-construct an `ArrayBuffer` around something that has been transferred. For all we know, that buffer is now being used by a different thread in a Web Worker.

Until the underlying byte source actually calls `.respond()` or `.respondWithNewView()` itself, we have no way to "reclaim" the transferred buffer.

> * ...and if it's not transferred, call `respond(0)`?

That *might* work, but it can cause unexpected behavior:
```javascript
const rs = new ReadableStream({
  type: "bytes",
  pull(controller) {
    const byobRequest = controller.byobRequest;
    if (byobRequest) {
      // Note: we're not returning this promise, so the stream will not wait for it before canceling.
      fetchSomeBytes(byobRequest.view.byteLength).then((data) => {
        // If the stream is canceled while fetching the data, then the stream would have already "auto-responded"
        // to the BYOB request. This invalidates the request: its buffer is transferred back to the stream,
        // and its view is set to null.
        byobRequest.view.set(data, 0); // 💥
        byobRequest.respond(data.byteLength);
      });
    }
  }
}
```

> * Relax the requirement that the last BYOB read should resolve with `value: emptyChunk`?

This would break [the `readInto()` example](https://streams.spec.whatwg.org/#example-manual-read-bytes). The BYOB reader *needs* to get its buffer back.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1114#discussion_r604490796