[whatwg/streams] Assert no pending reads on release (#1168)

When using `ReadableStream(Default|BYOB)Reader.releaseLock()`, we throw an error if there are still pending read requests ([spec](https://streams.spec.whatwg.org/commit-snapshots/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/#default-reader-release-lock)):

> The releaseLock() method steps are:
> 
> 1. If this.\[[stream]] is undefined, return.
> 2. If this.\[[readRequests]] is not empty, throw a TypeError exception.
> 3. Perform ! ReadableStreamReaderGenericRelease(this).

However, when we use the abstract op `ReadableStreamReaderGenericRelease` directly, we do not assert this. Some places in the spec often have an extra assert specifically for this (e.g. async iterator's return steps or `ReadableByteStreamTee`), but other places fail to perform this check (e.g. async iterator's next steps or [`ReadableStreamPipeTo`](https://streams.spec.whatwg.org/commit-snapshots/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/#ref-for-readable-stream-reader-generic-release%E2%91%A5)).

I added this missing check, but now the following WPT tests start failing as a result:
* [piping/abort.any.js](https://github.com/web-platform-tests/wpt/blob/87a4c80598aee5178c385628174f1832f5a28ad6/streams/piping/abort.any.js#L375): abort should do nothing after the writable is errored
* [piping/error-propagation-backward.any.js](https://github.com/web-platform-tests/wpt/blob/87a4c80598aee5178c385628174f1832f5a28ad6/streams/piping/error-propagation-backward.any.js#L419): Errors must be propagated backward: becomes errored after piping; preventCancel = true
* [readable-streams/async-iterator.any.js](https://github.com/web-platform-tests/wpt/blob/87a4c80598aee5178c385628174f1832f5a28ad6/streams/readable-streams/async-iterator.any.js#L626): return() should unlock the stream synchronously when preventCancel = true *(also when preventCancel = false)*

The async iterator test fails because we [release the lock in the middle of the read request's close steps](https://streams.spec.whatwg.org/commit-snapshots/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/#ref-for-readable-stream-reader-generic-release), but [`ReadableStreamClose`](https://streams.spec.whatwg.org/commit-snapshots/8bbfbf83ad06a8b9224bbe1cfbcbbbdbdd827a19/#readable-stream-close) only clears the list of read requests *after* calling all the close steps. The fix is quite simple: empty the list *before* calling the close steps or error steps. I'll push a commit for that on this PR.

The pipe tests fail because the pipe loop actually *starts a read request*, but we don't wait for that read to complete before releasing the lock. I *think* it's possible you could lose a chunk this way, but I haven't yet figured out how. I'll keep trying though. 😛 I think we can solve this one by tracking both `currentWrite` *and* `currentRead`, but I'm not sure about the details yet.

- [ ] 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/1168


-- Commit Summary --

  * <a href="https://github.com/whatwg/streams/pull/1168/commits/d51338f7d5854569ca58c6ac21294842b36e2cdc">Assert there are no pending reads when releasing lock</a>
  * <a href="https://github.com/whatwg/streams/pull/1168/commits/5ba4701dbb94243973d857f59d987c417d1b16fb">Update spec text</a>

-- File Changes --

    M index.bs (40)
    M reference-implementation/lib/ReadableStream-impl.js (8)
    M reference-implementation/lib/ReadableStreamBYOBReader-impl.js (2)
    M reference-implementation/lib/ReadableStreamDefaultReader-impl.js (2)
    M reference-implementation/lib/abstract-ops/readable-streams.js (20)

-- Patch Links --

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

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