Skip to content

Commit

Permalink
stream: update ongoing promise in async iterator return() method
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
MattiasBuelens authored and marco-ippolito committed Aug 19, 2024
1 parent 4a3ecbf commit 9b82b15
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 7 deletions.
4 changes: 3 additions & 1 deletion lib/internal/webstreams/readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; },
Expand Down
6 changes: 3 additions & 3 deletions test/fixtures/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
21 changes: 21 additions & 0 deletions test/fixtures/wpt/streams/piping/detached-context-crash.html
Original file line number Diff line number Diff line change
@@ -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>
86 changes: 84 additions & 2 deletions test/fixtures/wpt/streams/readable-streams/async-iterator.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
2 changes: 1 addition & 1 deletion test/fixtures/wpt/versions.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"path": "resources"
},
"streams": {
"commit": "3df6d94318b225845a0c8e4c7718484f41c9b8ce",
"commit": "9b03282a99ef2314c1c2d5050a105a74a2940019",
"path": "streams"
},
"url": {
Expand Down

0 comments on commit 9b82b15

Please sign in to comment.