Re: [whatwg/streams] ReadableStreamClose with async iterator and pending read requests errors (#1100)

> From what I can tell, ReadableStreamClose resolves the reader's [[closedPromise]] with undefined, and sets the [[state]] to "closed". So then the loop would repace [[closedPromise]] with a new rejected promise, I think. Is that what you mean?

Yup, that's correct.

> More generally, I'm wondering if this problem is reproducible outside the framework of async iterators. Isn't something like the following very similar?
>
> I guess that one is saved by the extra microtask delay, which effectively makes the release call happen after the state has already become "closed"?

Indeed, it's not reproducible with `reader.read()` because of the microtask delay.

Async iterators are a bit special, in that they can *synchronously* release the reader's lock in response to a read request. (But then again, this isn't really observable, since async iterators don't actually use `[[closedPromise]]`.)

It *could* be observable in other specifications though. When using the ["read a chunk" algorithm](https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-a-chunk), the close steps also run synchronously. So they *could* [observe that the stream is closed](https://streams.spec.whatwg.org/#readablestream-closed) while `reader.closed` is still pending. It's very unlikely any specification would actually do this, but it's *theoretically* possible. 😛 

> If that's the case, then maybe rearranging the steps would be better, so that the async iterator close steps behave similarly to such a code snippet...
>
> And also just from a general code hygeine perspective, ReadableStreamClose does look a little weird: it should probably keep [[state]] setting closer to [[closedPromise]] setting, without the arbitrary spec code of the close steps running in between.

Agreed. I think it's good practice to perform all the necessary updates to our own state first, and then perform any registered "callbacks" (close or error steps) at the end. I feel it makes things easier to reason about. 🙂

---

If you're okay with it, I can make a PR to move those steps around in the specification and update that single WPT test to expect a slightly different order. (Or alternatively: maybe we should make the test not check the order of `reader.read()` and `reader.closed` becoming rejected? It doesn't seem that important. 🤷‍♂️)

Oh, right, I'll also include my regression test case then. 😄

-- 
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/1100#issuecomment-761282151