[whatwg/streams] Resolve BYOB reads immediately on cancel (#1129)

Right now, if you cancel the stream while a BYOB read is pending, the stream has to wait for the underlying byte source to call `respond(0)` before it can return the BYOB request's buffer to the caller. Effectively, every single underlying byte source would need to look like this:
```javascript
const readable = new ReadableStream({
  type: 'bytes',
  autoAllocateChunkSize: 1024,
  start(controller) {
    this._controller = controller;
  },
  async pull(controller) {
    const bytesWritten = await readBytesIntoViewSomehow(controller.byobRequest.view);
    controller.byobRequest.respond(bytesWritten);
  },
  cancel(reason) {
    this._controller.byobRequest?.respond(0); // must not forget this!
  }
});
```
This is a footgun: we *have* to rely on the underlying byte source to do this correctly. We are unable to do this automatically, since we don't know if or how the source might be using the BYOB request (for example, it might have transferred the buffer). If the source fails to call `respond(0)`, then the BYOB read never resolves and the reader gets stuck. If you forget to implement `cancel()` altogether, then the default implementation will not help either, since it is specified to do nothing.

In [#1114 (comment)](https://github.com/whatwg/streams/pull/1114#discussion_r605259271) and [#1123 (comment)](https://github.com/whatwg/streams/pull/1123#issuecomment-841413902), @domenic suggested that if you cancel the stream while a BYOB read is pending, the stream should simply not give you back your buffer. This means that `cancel()` can immediately resolve all pending BYOB reads with the classic `{ done: true, value: undefined }`, without having to wait for the underlying byte source. This solves the problem, and would make it easier to implement an underlying byte source.

This PR implements this suggestion. To make it work, I also had to change one other thing: when the stream is cancelled, any pending BYOB request is now *immediately invalidated*, so the underlying byte source doesn't erroneously think that it still needs to provide a response. (This aligns with the behavior of `controller.enqueue()`, which throws if the stream is already closed.)

- [ ] At least two implementers are interested (and none opposed):
   * …
   * …
- [x] [Tests](https://github.com/web-platform-tests/wpt) are written and can be reviewed and commented upon at:
   * web-platform-tests/wpt#29128
- [ ] [Implementation bugs](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) are filed:
   * Chrome: …
   * Firefox: …
   * Safari: …

(See [WHATWG Working Mode: Changes](https://whatwg.org/working-mode#changes) for more details.)

You can view, comment on, or merge this pull request online at:

  https://github.com/whatwg/streams/pull/1129


-- Commit Summary --

  * Resolve BYOB reads immediately on cancel
  * Roll WPT

-- File Changes --

    M index.bs (24)
    M reference-implementation/lib/ReadableByteStreamController-impl.js (5)
    M reference-implementation/lib/abstract-ops/readable-streams.js (9)
    M reference-implementation/web-platform-tests (2)

-- Patch Links --

https://github.com/whatwg/streams/pull/1129.patch

https://github.com/whatwg/streams/pull/1129.diff


-- 
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/1129

Received on Wednesday, 26 May 2021 21:34:53 UTC