From eee9db8b8ed2fd7060127e5b996b8262c2ae16a7 Mon Sep 17 00:00:00 2001 From: Mattias Buelens <649348+MattiasBuelens@users.noreply.github.com> Date: Sun, 28 Apr 2024 18:51:44 +0200 Subject: [PATCH] stream: update ongoing promise in async iterator return() method PR-URL: https://github.com/nodejs/node/pull/52657 Reviewed-By: Matteo Collina Reviewed-By: Debadree Chatterjee Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- lib/internal/webstreams/readablestream.js | 4 +- test/fixtures/wpt/README.md | 2 +- .../crashtests/cross-piping2.https.html | 14 +++ .../piping/detached-context-crash.html | 21 +++++ .../readable-streams/async-iterator.any.js | 86 ++++++++++++++++++- .../tee-detached-context-crash.html | 13 +++ test/fixtures/wpt/versions.json | 2 +- 7 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html create mode 100644 test/fixtures/wpt/streams/piping/detached-context-crash.html create mode 100644 test/fixtures/wpt/streams/readable-streams/tee-detached-context-crash.html diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index dbf30326d1e97b..c1bf9909ee1449 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -545,12 +545,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 index 3a2bb782480ef6..6808a9c804d49e 100644 --- a/test/fixtures/wpt/README.md +++ b/test/fixtures/wpt/README.md @@ -27,7 +27,7 @@ Last update: - 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/1e140d63ec/resources -- streams: https://github.com/web-platform-tests/wpt/tree/3df6d94318/streams +- 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 diff --git a/test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html b/test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html new file mode 100644 index 00000000000000..4616aef479c07b --- /dev/null +++ b/test/fixtures/wpt/streams/piping/crashtests/cross-piping2.https.html @@ -0,0 +1,14 @@ + + diff --git a/test/fixtures/wpt/streams/piping/detached-context-crash.html b/test/fixtures/wpt/streams/piping/detached-context-crash.html new file mode 100644 index 00000000000000..56271f12c3ae76 --- /dev/null +++ b/test/fixtures/wpt/streams/piping/detached-context-crash.html @@ -0,0 +1,21 @@ + + + + diff --git a/test/fixtures/wpt/streams/readable-streams/async-iterator.any.js b/test/fixtures/wpt/streams/readable-streams/async-iterator.any.js index 4b674bea8430f3..e192201b531da7 100644 --- 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 new file mode 100644 index 00000000000000..9488da72732e00 --- /dev/null +++ b/test/fixtures/wpt/streams/readable-streams/tee-detached-context-crash.html @@ -0,0 +1,13 @@ + + + + diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json index 5f65dd1a829c58..8da73d9dd85d91 100644 --- a/test/fixtures/wpt/versions.json +++ b/test/fixtures/wpt/versions.json @@ -68,7 +68,7 @@ "path": "resources" }, "streams": { - "commit": "3df6d94318b225845a0c8e4c7718484f41c9b8ce", + "commit": "9b03282a99ef2314c1c2d5050a105a74a2940019", "path": "streams" }, "url": {