stream: update ongoing promise in async iterator return() method

PR-URL: #52657 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Aug 19, 2024 · 9b82b15 · 9b82b15
1 parent 4a3ecbf
commit 9b82b15
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 7 deletions.
diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
@@ -548,12 +548,14 @@ class ReadableStream {
       },
 
       return(error) {
-        return state.current ?
+        started = true;
+        state.current = state.current !== undefined ?
           PromisePrototypeThen(
             state.current,
             () => returnSteps(error),
             () => returnSteps(error)) :
           returnSteps(error);
+        return state.current;
       },
 
       [SymbolAsyncIterator]() { return this; },

diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md
@@ -25,9 +25,9 @@ Last update:
 - interfaces: https://github.com/web-platform-tests/wpt/tree/df731dab88/interfaces
 - performance-timeline: https://github.com/web-platform-tests/wpt/tree/17ebc3aea0/performance-timeline
 - resource-timing: https://github.com/web-platform-tests/wpt/tree/22d38586d0/resource-timing
-- resources: https://github.com/web-platform-tests/wpt/tree/919874f84f/resources
-- streams: https://github.com/web-platform-tests/wpt/tree/3df6d94318/streams
-- url: https://github.com/web-platform-tests/wpt/tree/c2d7e70b52/url
+- resources: https://github.com/web-platform-tests/wpt/tree/1e140d63ec/resources
+- streams: https://github.com/web-platform-tests/wpt/tree/9b03282a99/streams
+- url: https://github.com/web-platform-tests/wpt/tree/0f550ab9f5/url
 - user-timing: https://github.com/web-platform-tests/wpt/tree/5ae85bf826/user-timing
 - wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi
 - wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi

diff --git a/test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html b/test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<script type="module">
+  let a = self.open()
+  let d = await a.navigator.storage.getDirectory()
+  let h = await d.getFileHandle("c5c9960b-a637-4232-be3d-3ccc5704852f", {"create": true})
+  let r = new ReadableStream({
+    start(c) {
+      c.enqueue(h)
+      c.close();
+    }
+  });
+  let w = await h.createWritable({ })
+  r.pipeThrough({"readable": r, "writable": w}, {})
+</script>
diff --git a/test/fixtures/wpt/streams/piping/detached-context-crash.html b/test/fixtures/wpt/streams/piping/detached-context-crash.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<body>
+<script>
+window.onload = () => {
+  const i = document.createElement("iframe");
+  i.src = "about:blank";
+  document.body.appendChild(i);
+
+  const rs = new i.contentWindow.ReadableStream({
+    start(controller) { controller.error(); }
+  });
+  const ws = new i.contentWindow.WritableStream();
+
+  i.remove();
+
+  // pipeTo() should not crash with a ReadableStream or WritableStream from
+  // a detached iframe.
+  rs.pipeTo(ws);
+};
+</script>
+</body>
diff --git a/test/fixtures/wpt/streams/readable-streams/async-iterator.any.js b/test/fixtures/wpt/streams/readable-streams/async-iterator.any.js
@@ -475,15 +475,85 @@ promise_test(async () => {
   const rs = new ReadableStream();
   const it = rs.values();
 
-  const iterResults = await Promise.allSettled([it.return('return value'), it.next()]);
+  const resolveOrder = [];
+  const iterResults = await Promise.allSettled([
+    it.return('return value').then(result => {
+      resolveOrder.push('return');
+      return result;
+    }),
+    it.next().then(result => {
+      resolveOrder.push('next');
+      return result;
+    })
+  ]);
 
   assert_equals(iterResults[0].status, 'fulfilled', 'return() promise status');
   assert_iter_result(iterResults[0].value, 'return value', true, 'return()');
 
   assert_equals(iterResults[1].status, 'fulfilled', 'next() promise status');
   assert_iter_result(iterResults[1].value, undefined, true, 'next()');
+
+  assert_array_equals(resolveOrder, ['return', 'next'], 'next() resolves after return()');
 }, 'return(); next() [no awaiting]');
 
+promise_test(async () => {
+  let resolveCancelPromise;
+  const rs = recordingReadableStream({
+    cancel(reason) {
+      return new Promise(r => resolveCancelPromise = r);
+    }
+  });
+  const it = rs.values();
+
+  let returnResolved = false;
+  const returnPromise = it.return('return value').then(result => {
+    returnResolved = true;
+    return result;
+  });
+  await flushAsyncEvents();
+  assert_false(returnResolved, 'return() should not resolve while cancel() promise is pending');
+
+  resolveCancelPromise();
+  const iterResult1 = await returnPromise;
+  assert_iter_result(iterResult1, 'return value', true, 'return()');
+
+  const iterResult2 = await it.next();
+  assert_iter_result(iterResult2, undefined, true, 'next()');
+}, 'return(); next() with delayed cancel()');
+
+promise_test(async () => {
+  let resolveCancelPromise;
+  const rs = recordingReadableStream({
+    cancel(reason) {
+      return new Promise(r => resolveCancelPromise = r);
+    }
+  });
+  const it = rs.values();
+
+  const resolveOrder = [];
+  const returnPromise = it.return('return value').then(result => {
+    resolveOrder.push('return');
+    return result;
+  });
+  const nextPromise = it.next().then(result => {
+    resolveOrder.push('next');
+    return result;
+  });
+
+  assert_array_equals(rs.events, ['cancel', 'return value'], 'return() should call cancel()');
+  assert_array_equals(resolveOrder, [], 'return() should not resolve before cancel() resolves');
+
+  resolveCancelPromise();
+  const iterResult1 = await returnPromise;
+  assert_iter_result(iterResult1, 'return value', true, 'return() should resolve with original reason');
+  const iterResult2 = await nextPromise;
+  assert_iter_result(iterResult2, undefined, true, 'next() should resolve with done result');
+
+  assert_array_equals(rs.events, ['cancel', 'return value'], 'no pull() after cancel()');
+  assert_array_equals(resolveOrder, ['return', 'next'], 'next() should resolve after return() resolves');
+
+}, 'return(); next() with delayed cancel() [no awaiting]');
+
 promise_test(async () => {
   const rs = new ReadableStream();
   const it = rs.values();
@@ -499,13 +569,25 @@ promise_test(async () => {
   const rs = new ReadableStream();
   const it = rs.values();
 
-  const iterResults = await Promise.allSettled([it.return('return value 1'), it.return('return value 2')]);
+  const resolveOrder = [];
+  const iterResults = await Promise.allSettled([
+    it.return('return value 1').then(result => {
+      resolveOrder.push('return 1');
+      return result;
+    }),
+    it.return('return value 2').then(result => {
+      resolveOrder.push('return 2');
+      return result;
+    })
+  ]);
 
   assert_equals(iterResults[0].status, 'fulfilled', '1st return() promise status');
   assert_iter_result(iterResults[0].value, 'return value 1', true, '1st return()');
 
   assert_equals(iterResults[1].status, 'fulfilled', '2nd return() promise status');
   assert_iter_result(iterResults[1].value, 'return value 2', true, '1st return()');
+
+  assert_array_equals(resolveOrder, ['return 1', 'return 2'], '2nd return() resolves after 1st return()');
 }, 'return(); return() [no awaiting]');
 
 test(() => {

diff --git a/test/fixtures/wpt/streams/readable-streams/tee-detached-context-crash.html b/test/fixtures/wpt/streams/readable-streams/tee-detached-context-crash.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<body>
+<script>
+const i = document.createElement("iframe");
+document.body.appendChild(i);
+
+const rs = new i.contentWindow.ReadableStream();
+i.remove();
+
+// tee() on a ReadableStream from a detached iframe should not crash.
+rs.tee();
+</script>
+</body>
diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json
@@ -64,7 +64,7 @@
     "path": "resources"
   },
   "streams": {
-    "commit": "3df6d94318b225845a0c8e4c7718484f41c9b8ce",
+    "commit": "9b03282a99ef2314c1c2d5050a105a74a2940019",
     "path": "streams"
   },
   "url": {