Fix two of E18's three close-flush data-loss bugs - #116
Closed
lann wants to merge 1 commit into
Closed
Conversation
The libwebrtc reference peer's immediate close-after-send coalesces the tail DATA chunks and the SCTP stream-reset RECONFIG into one network burst; three distinct receiver-side bugs mishandled it. This fixes two: - Pin rtc to lann/rtc rev 353a84a (upstream a10cd2c plus the cherry-pick of lann/rtc#1): rtc-sctp no longer discards received-but-undelivered messages when an incoming stream reset arrives in the same input batch as the data — stream teardown is deferred until the consumer drains the reassembly queue. - The reference peer's wait_open now treats an already-closing remote-announced channel as opened: announcement implies the open transition happened, and its messages are already queued in the wiring, so a peer landing open→send→close inside one 25ms poll interval is not an error. Also make recv_sequence failures in the conformance guest and the reference peer report how many messages had arrived, distinguishing partial delivery from a receive path that never yielded. The third bug — the webrtc 0.20 driver delivering a data-channel close ahead of already-received messages (poll_events runs before poll_reads; close and data travel different queues) — remains; TODO E18 records the full diagnosis. Verified with 20-iteration channel-close-flush interop loops (reference-x-wasmtime + wasmtime-x-reference): the wait_open failure and the rtc-sctp loss no longer reproduce; remaining failures are all the webrtc driver reorder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Superseded by #139, which ports this onto main (post-#135/#137 tree): same two fixes and diagnostics; the rtc fix now rides pinned git dependencies (lann/rtc#1 rebased to 89f518ea, consumed through a lann/webrtc submodule-redirect branch) instead of [patch.crates-io], since the rebase sits on unreleased 0.21.0 and webrtc master vendors rtc as a submodule path dependency. |
lann
added a commit
that referenced
this pull request
Aug 6, 2026
Fix two of the close-flush data-loss bugs (#116 ported to main)
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
1 participant
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.
E18 is now fully diagnosed: the libwebrtc reference peer is blameless. Its immediate close-after-send (spec-legal; libwebrtc flushes before the SCTP reset per RFC 8831 §6.7) coalesces the tail DATA and the reset RECONFIG into one burst, and three distinct receiver-side bugs mishandled it. This PR fixes two and documents all three in TODO E18.
Fixed here
rtc-sctpdiscarded received-but-undelivered messages on an incoming stream reset (reference-x-wasmtime,answerer: receive: closed).reset_streams_if_any→unregister_streamdropped the stream's reassembly queue with reassembled-but-unread messages inside, before the driver polled the pendingReadableevents. Fixed by sctp: don't discard received-but-undelivered data on incoming stream reset lann/rtc#1; the[patch.crates-io]pin moves tolann/rtcrev353a84a= upstreama10cd2c(the existing E4/E6 srflx fix) + that cherry-pick, so nothing is lost.wait_openrejected an already-closing incoming channel (wasmtime-x-reference,answerer: channel closed before open). A remote-announced channel that readsclosing/closedwas necessarily open first and its messages are already queued; open→send→close can land inside one 25ms poll interval. Remote channels now count that state as opened; locally created channels still fail.Also:
recv_sequencefailures (conformance guest + reference peer) now report(after N of M messages), which is what made the diagnosis tractable.Remaining (tracked in E18, not fixed here)
webrtc0.20 driver delivers a data-channel close ahead of already-received messages: its loop runspoll_events()beforepoll_reads(), and close/data travel different core queues. Fix belongs upstream inwebrtc-rs/webrtc.reference-x-jco-nodefailures with the same trigger are E16 (node-datachannel TSFN race).Verification
20-iteration
channel-close-flushloops onreference-x-wasmtime+wasmtime-x-reference: bugs 1 and 2 no longer reproduce (0/40 for the wait_open shape; SCTP traces show all payloads reaching the driver); remaining failures (4/20 iterations) are all bug 3's shapes.just fmt-check,just clippy,just testgreen.Supersedes #115.