Re: [whatwg/streams] Semantics of |closedOrErrored| variable in ReadableStreamTee are odd (#991)

> |closedOrErrored| can't actually be true when this is called, because it is only set to true when the stream is closed or errored, but if the stream is closed then it won't be errored.
> 
> Similarly, when |closedOrErrored| is set because |done| was true, we know that it can't have been set due to an error, because the call to DefaultReaderRead would have rejected in that case.

That seems correct to me.

> My suggestion is to rename |closedOrErrored| to just |closed| and only look at or modify it for the closed case.

Yup, I think that should work.

> But it may be possible to eliminate the variable altogether.

I don't think we can eliminate it.

Both branches use the same `pullAlgorithm`, and it's possible that they both pull at the same time. Both pulls call `DefaultReaderRead`, so both could see `{done: true}`. The first read will close the two branches and set the `closedOrErrored` (or just `closed`) flag to `true`, so that the second read doesn't attempt to close them again. You can see this in action with the _"ReadableStream teeing: closing the original should immediately close the branches"_ test.

-----

Actually... isn't that a bug with `pullAlgorithm`? For a single readable stream, we guarantee that we won't call `pull()` until the previous pull promise has resolved. But in the case of `tee()`, we pass *the same pull algorithm* to both streams, so there's nothing stopping them from calling that function "concurrently". Since both branches have a high water mark of 1, they'll start out calling `pull()`, so we will start **2 read operations instead of just 1**!

Demo:
```js
let controller;
let remainingChunks = ['a', 'b', 'c'];
const rs = new ReadableStream({
  start(c) {
    controller = c;
  },
  pull(c) {
    console.log('enqueue', remainingChunks[0]);
    c.enqueue(remainingChunks.shift());
    if (remainingChunks.length === 0) {
      c.close();
    }
  }
}, {highWaterMark: 0});
```
**Expected output**
```
enqueue a
```
(Both branches have HWM = 1, so `tee()` should read one chunk and enqueue it on both branches.)

**Actual output**
```
enqueue a
enqueue b
```
(Both branches call `pull()` at the same time, so we read two chunks and enqueue them on both branches. This pushes them over their HWM = 1.)

...I guess I should probably file this as a separate issue? 😛 

-- 
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/issues/991#issuecomment-465814861