Re: [whatwg/streams] Aborting a stream should wait for pending writes (#619)

domenic approved this pull request.

LGTM with nits. Thanks so much.

> @@ -2614,6 +2614,18 @@ Instances of {{WritableStream}} are created with the internal slots described in
     <td>\[[writeRequests]]
     <td>A List of promises representing the stream's internal queue of pending writes
   </tr>
+  <tr>
+    <td>\[[pendingWriteRequest]]
+    <td>The promise for the current pending write operation
+  </tr>
+  <tr>
+    <td>\[[pendingCloseRequest]]
+    <td>The promise returned from the writer close method

`{{ReadableStreamDefaultWriter/close()}` instead of just `close` probably

> @@ -2759,6 +2771,14 @@ writable stream is <a>locked to a writer</a>.
   1. Assert: _state_ is `"writable"` or `"closing"`.
   1. Let _error_ be a new *TypeError* indicating that the stream has been aborted.
   1. Perform ! WritableStreamError(_stream_, _error_).
+  1. Let _controller_ be _stream_.[[writableStreamController]].
+  1. Assert: _controller_ is not *undefined*.
+  1. If _controller_.[[writing]] is *true* or _controller_.[[inClose]] is *true*,
+    1. Set _stream_.[[pendingAbortRequest]] to <a>a new promise</a>.
+    1. If _controller_.[[writing]] is *true*, return the result of transforming _stream_.[[pendingAbortRequest]] by a
+    fulfillment handler that returns
+    ! WritableStreamDefaultControllerAbort(_stream_.[[writableStreamController]], _reason_).

! on previous line (the idea is to make the wrapping machine-enforceable, at least in theory)

> @@ -2789,32 +2809,58 @@ visible through the {{WritableStream}}'s public API.
 )</h4>
 
 <emu-alg>
-  1. Let _state_ be _stream_.[[state]].
-  1. Assert: _state_ is `"writable"` or `"closing"`.
-  1. Repeat for each _writeRequest_ that is an element of _stream_.[[writeRequests]],
-    1. <a>Reject</a> _writeRequest_ with _e_.
-  1. Set _stream_.[[writeRequests]] to an empty List.
+  1. Let _oldState_ be _stream_.[[state]].
+  1. Assert: _oldState_ is `"writable"` or `"closing".`

backtick before period

> +    1. <a>Resolve</a> _stream_.[[pendingAbortRequest]] with *undefined*.
+    1. Set _stream_.[[pendingAbortRequest]] to *undefined*.
+</emu-alg>
+
+<h4 id="writable-stream-reject-unresolved-promises" aoid="WritableStreamRejectUnresolvedPromises"
+nothrow>WritableStreamRejectUnresolvedPromises ( <var>stream</var> )</h4>
+
+<emu-alg>
+  1. Assert: _stream_.[[state]] is `"errored"`.
+  1. Assert: _stream_.[[pendingWriteRequest]] is *undefined*.
+  1. Let _storedError_ be _stream_.[[storedError]].
+  1. Repeat for each _writeRequest_ that is an element of _stream_.[[writeRequests]],
+    1. <a>Reject</a> _writeRequest_ with _storedError_.
+  1. Set _stream_.[[writeRequests]] to an empty List.
+  1. If _stream_.[[pendingCloseRequest]] is not *undefined*,
+    1. Assert: _stream_.[[writableStreamController]].[[inClose]] is *false*,

period not comma

> @@ -3213,6 +3259,11 @@ Instances of {{WritableStreamDefaultController}} are created with the internal s
     <td>A boolean flag set to <emu-val>true</emu-val> while the <a>underlying sink</a>'s <code>write</code> method is
       executing and has not yet fulfilled, used to prevent reentrant calls
   </tr>
+  <tr>
+    <td>\[[inClose]]
+    <td>A boolean flag set to <emu-val>true</emu-val> while the <a>underlying sink</a>'s <code>close</code> method is
+      executing and has not yet fulfilled, used to prevent the <code>abort</code> method from

`{{WritableStreamDefaultWriter/abort()}}`

>    1. Let _sinkClosePromise_ be ! PromiseInvokeOrNoop(_controller_.[[underlyingSink]], `"close"`, « ‍_controller_ »).
   1. <a>Upon fulfillment</a> of _sinkClosePromise_,
-    1. If _stream_.[[state]] is not `"closing"`, return.
-    1. Perform ! WritableStreamFulfillWriteRequest(_stream_).
+    1. Assert: _controller_.[[inClose]] is *true*.
+    1. Set  _controller_.[[InClose]] to *false*.

Lowercase InClose

> @@ -173,11 +176,26 @@ function WritableStreamFinishClose(stream) {
   defaultWriterClosedPromiseResolve(stream._writer);
 }
 
-function WritableStreamFulfillWriteRequest(stream) {
-  assert(stream._writeRequests.length > 0);
+function WritableStreamRejectUnresolvedPromises(stream) {

WritableStreamRejectPromisesInReactionToError? Not important, but maybe an improvement.

-- 
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/619#pullrequestreview-11266963