Skip to content

Always resolve tee's cancel promise after stream closes or errors#1039

Merged
domenic merged 4 commits into
whatwg:masterfrom
MattiasBuelens:tee-cancel-bug
Aug 5, 2020
Merged

Always resolve tee's cancel promise after stream closes or errors#1039
domenic merged 4 commits into
whatwg:masterfrom
MattiasBuelens:tee-cancel-bug

Conversation

@MattiasBuelens

@MattiasBuelens MattiasBuelens commented Jun 11, 2020

Copy link
Copy Markdown
Collaborator

@MattiasBuelens

Copy link
Copy Markdown
Collaborator Author

Keeping this as a draft for now.

I think that a similar issue could occur when the original stream errors while one branch has started cancelling. I suspect that would also keep the cancel promise pending indefinitely, but I first need to write a test to prove it.

@MattiasBuelens
MattiasBuelens force-pushed the tee-cancel-bug branch 2 times, most recently from 89d031b to 8dc6995 Compare June 12, 2020 20:27
Comment thread index.bs
@MattiasBuelens
MattiasBuelens marked this pull request as ready for review June 12, 2020 20:45
@MattiasBuelens MattiasBuelens changed the title Always resolve tee's cancel promise after reaching end of stream Always resolve tee's cancel promise after stream closes or errors Jun 12, 2020
@MattiasBuelens
MattiasBuelens force-pushed the tee-cancel-bug branch 2 times, most recently from 79a0dd5 to 32b4f9f Compare July 24, 2020 20:30
@MattiasBuelens

MattiasBuelens commented Jul 24, 2020

Copy link
Copy Markdown
Collaborator Author

I think this is ready for review.

There were some other issues with WPT which also affected this PR, hence the influx of pull requests. Now that those are fixed, this PR is finally green again. 😄

@domenic domenic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. @ricea, I assume Chromium is interested in this fix?

@jorendorff and @youennf, does this sound good to you? I know traditionally we haven't been asking for multi-implementer support for every bugfix to streams, but I think it'd be good for us to get in that habit.

@ricea

ricea commented Jul 28, 2020

Copy link
Copy Markdown
Collaborator

Yes, speaking for Google Chrome, we want this fix.

@domenic

domenic commented Aug 5, 2020

Copy link
Copy Markdown
Member

@MattiasBuelens would you mind rebasing? @youennf confirmed he's good with this fix, so we can merge once that's done.

domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 5, 2020
@domenic
domenic merged commit 6cd5e81 into whatwg:master Aug 5, 2020
@MattiasBuelens
MattiasBuelens deleted the tee-cancel-bug branch August 5, 2020 22:34
@domenic

domenic commented Aug 5, 2020

Copy link
Copy Markdown
Member

I filed bugs for WebKit and Gecko, and updated the OP with links to them. For Chromium we'll automatically get a bug filed by the web platform tests importer, and I know @ricea has been pretty good about triaging those.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 8, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 15, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: d835b5e68f7177a62bd9782ba6447e786ddac6c9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: f12c531e015c22fc03483b942fdf6e15e9ad686f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: d835b5e68f7177a62bd9782ba6447e786ddac6c9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: f12c531e015c22fc03483b942fdf6e15e9ad686f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: d835b5e68f7177a62bd9782ba6447e786ddac6c9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Aug 16, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124

UltraBlame original commit: f12c531e015c22fc03483b942fdf6e15e9ad686f
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Aug 17, 2020
Always resolve tee's cancel promise after stream closes or errors.
Applies whatwg/streams#1039 to our
implementation.

BUG=1114014

Change-Id: Ic6d665bd928553ea3a292fe8221f0812d1c613e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358929
Reviewed-by: Yutaka Hirano <[email protected]>
Commit-Queue: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/master@{#798520}
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this pull request Sep 23, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
ambroff pushed a commit to ambroff/gecko that referenced this pull request Nov 4, 2020
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…tream closes or errors, a=testonly

Automatic update from web-platform-tests
Test canceling one branch when stream closes or errors

Follows whatwg/streams#1039.
--

wpt-commits: e5e5e7a10cbb968b31c51ad87ce8a3da62bb1b34
wpt-pr: 24124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Cancelling one branch of a tee'd stream never finishes if other branch reads all chunks

3 participants