Repository navigation
WebRTC: give up what a peer never forwards (the loss test's stall) - #348
Closed
NoahSurprenant wants to merge 2 commits into
Closed
NoahSurprenant wants to merge 2 commits into
NoahSurprenant wants to merge 2 commits into
Conversation
The web job's "with losses" case (port/web/tests/webrtc_native.mjs) failed now and then with the channel open but nothing echoed after about a hundred messages. The cause, found with packet traces of the native end: when the first DATA chunk a browser sends is lost, Firefox's SCTP (usrsctp) sends one FORWARD-TSN for it and never another, so the next loss leaves this end's cumulative TSN stuck below it for good. The SACKs' gaps then soon pass MAXIMUM_GAP_BLOCKS, what they leave out stays in the browser's flight, its congestion window fills and it sends nothing more. Losing the first DATA on purpose stalled 6 of 6 runs (and a later one 0 of 6), so it fails about as often as the test's loss rate, 10%; worse under load. A real player on a lossy network would hit it the same way. The channel sends nothing twice (negotiated, maxRetransmits 0), so what is missing below a TSN that arrived GAP_TIME (250 ms) ago will never come: this end now gives it up as the peer's FORWARD-TSN would have, and SACKs. The loss case, 40 runs per browser, two at a time: Firefox 9 of 40 failed before, 0 of 40 after; Chromium 0 of 40 after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 76352bc)
…CommunityEdition#348) The loss case caught the stall the previous commit fixes only when its random losses happened to take the browser's first DATA chunk, about one run in ten. The new case takes it every time: the harness (webrtc_native.c, linked with -Wl,--wrap=posix_dtls_receive) removes the DATA chunks HARNESS_DROP names from the SCTP packets p2p_webrtc.c reads, and the test has it lose the first and every tenth after it (31 gaps, more than the 16 a SACK carries), and nothing else. The other messages must come back, to the end. Checked with Playwright 1.58: without the previous commit the case failed 10 of 10 runs in Firefox (96 of 320 messages back, none after the 117th); with it, it passed 10 of 10 in Firefox and 10 of 10 in Chromium (289 of 320). Chromium's SCTP does not stall this way and passes either way (10 of 10 without the fix). Losing every tenth chunk but not the first passes in Firefox without the fix too. The case takes about five seconds a browser; node port/web/tests/webrtc_native.mjs chromium firefox passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NoahSurprenant
added a commit
to NoahSurprenant/OpenCE
that referenced
this pull request
Oct 14, 2026
…CommunityEdition#348) (#39) The loss case caught the stall the previous commit fixes only when its random losses happened to take the browser's first DATA chunk, about one run in ten. The new case takes it every time: the harness (webrtc_native.c, linked with -Wl,--wrap=posix_dtls_receive) removes the DATA chunks HARNESS_DROP names from the SCTP packets p2p_webrtc.c reads, and the test has it lose the first and every tenth after it (31 gaps, more than the 16 a SACK carries), and nothing else. The other messages must come back, to the end. Checked with Playwright 1.58: without the previous commit the case failed 10 of 10 runs in Firefox (96 of 320 messages back, none after the 117th); with it, it passed 10 of 10 in Firefox and 10 of 10 in Chromium (289 of 320). Chromium's SCTP does not stall this way and passes either way (10 of 10 without the fix). Losing every tenth chunk but not the first passes in Firefox without the fix too. The case takes about five seconds a browser; node port/web/tests/webrtc_native.mjs chromium firefox passes. (cherry picked from commit b7715ce) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
port/web/tests/webrtc_native.mjs's "with losses" case fails now and then: the data channel stays open but nothing more is echoed after about a hundred messages (e.g.firefox with losses: {"sent":960,"echoed":72,"wrong":0,"lastEchoed":100,"open":true}, and the same with Chromium on a loaded runner).Cause (found with packet traces of the native end): when the first DATA chunk a browser sends is lost, Firefox's SCTP (usrsctp) sends one FORWARD-TSN for it and never another, so the next loss leaves the native end's cumulative TSN stuck below it for good. The SACKs' gaps then soon pass
MAXIMUM_GAP_BLOCKS, what they leave out stays counted in the browser's flight, its congestion window fills and it stops sending. Dropping the browser's first DATA on purpose stalled 6 of 6 runs (dropping a later one, 0 of 6), so the test fails at about its loss rate (10%), more under load. A real player on a lossy network hits it the same way.Fix (
port/linux/src/p2p_webrtc.conly): the channel never sends anything twice (negotiated,maxRetransmits: 0), so whatever is still missing below a TSN that arrivedGAP_TIME(250 ms) ago will never come. The native end now gives it up itself, as the peer's FORWARD-TSN would have, and SACKs, instead of depending on the peer's FORWARD-TSN being sent and arriving.Checked (the loss case of
webrtc_native.mjs, Playwright 1.58):The commit is cherry-picked (
-x) from a fork where it has been in CI since.Regression test (second commit,
port/web/tests/webrtc_native.mjsandwebrtc_native.conly): a new case, "the first DATA chunk lost and every tenth", loses those chunks of the browser's and nothing else, every run, so the stall no longer depends on the random losses taking the first one. The harness is linked with-Wl,--wrap=posix_dtls_receiveand removes the DATA chunksHARNESS_DROPnames from the SCTP packetsp2p_webrtc.creads (recomputing the CRC-32C); the test loses the 1st, 10th, 20th … 300th (31 gaps, more than the 16 a SACK carries) and requires the rest back, to the end. It takes about five seconds a browser and runs in thewebjob with the other cases (node port/web/tests/webrtc_native.mjs chromium firefox).Chromium does not stall this way and passes either way. Losing every tenth chunk but not the first passes in Firefox without the fix too. With the fix,
node port/web/tests/webrtc_native.mjs chromium firefoxpasses; without it, it fails on the new case in Firefox.🤖 Generated with Claude Code