[whatwg/streams] Fix tee() incorrectly closing before enqueuing to the second branch (#1172)

I found a case where `tee()` inadvertently drops a chunk for the second branch, even though neither of the branches have been canceled. This is obviously very bad, because both branches *should* receive all enqueued chunks.

See \[insert WPT PR link here] for the problematic test case. Here's how it breaks down:
1. We call `controller.enqueue('a')`. Since there is a pending read request from `tee()` on the original stream, we synchronously run [its `chunkSteps`](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L368). However, this first calls `queueMicrotask()`, so the remainder will happen later.
2. We call `controller.close()`. Since we've already processed all pending read requests, the stream immediately moves to `state = 'closed'`.
3. One microtick passes. We run the remainder of `chunkSteps`.
    1. We set `reading = false`.
    2. [We call `ReadableStreamDefaultControllerEnqueue` for branch1.](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L391) Since there *is* a pending read request for branch1, it synchronously fulfills the request. And since we created both branches with the default HWM = 1, `ReadableStreamDefaultControllerShouldCallPull` is also true. So we call branch1's `pullAlgorithm`.
        1. We're back [at the start of `pullAlgorithm`](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L361). However, we've already set `reading = false`, we *do* start a new read request on the original stream.
        2. But the original stream is already closed, so [it *synchronously* runs `closeSteps`](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L912-L913).
        3. This brings us [back here](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L401-L406). We immediately close both branches.
     3. But wait a second. We're still in `chunkSteps`. So [we call `ReadableStreamDefaultControllerEnqueue` for branch2](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L395)... except, it's already closed, so [the chunk is ignored](https://github.com/whatwg/streams/blob/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/reference-implementation/lib/abstract-ops/readable-streams.js#L1026-L1028)! 😱💥

This PR fixes it by keeping `reading = true` until *after* we've enqueued/responded to both branches. Afterwards, we check if one of the two branches tried to call `pull()` (through an extra `readAgain` flag), and then call `pull()` ourselves. When teeing a readable byte stream, we also have to keep track *which* of the two branches called `pull()`, so we can use the appropriate BYOB request if available.

- [ ] At least two implementers are interested (and none opposed):
   * …
   * …
- [ ] [Tests](https://github.com/web-platform-tests/wpt) are written and can be reviewed and commented upon at:
   * …
- [ ] [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/1172


-- Commit Summary --

  * <a href="https://github.com/whatwg/streams/pull/1172/commits/3014563d64b091cffef2633dc9d2d614216e6228">Fix tee() incorrectly closing before enqueuing to the second branch</a>
  * <a href="https://github.com/whatwg/streams/pull/1172/commits/b97893f9302a821ea16195aa5e4b4c9299258c93">Roll WPT</a>

-- File Changes --

    M reference-implementation/lib/abstract-ops/readable-streams.js (34)
    M reference-implementation/web-platform-tests (2)

-- Patch Links --

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

https://github.com/whatwg/streams/pull/1172.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/1172