Skip to content

fix(webrtc): preserve read half on STOP_SENDING frames - #6577

Open
racequite wants to merge 1 commit into
libp2p:masterfrom
racequite:fix-webrtc-stop-sending-read-half
Open

fix(webrtc): preserve read half on STOP_SENDING frames#6577
racequite wants to merge 1 commit into
libp2p:masterfrom
racequite:fix-webrtc-stop-sending-read-half

Conversation

@racequite

Copy link
Copy Markdown

Description

Preserve WebRTC stream half-close semantics when processing control frames.

A payload-less STOP_SENDING frame previously caused poll_read to return Ok(0), incorrectly signaling EOF even though STOP_SENDING only closes the local write half. Continue polling for inbound data after handling this flag.

Report RESET as ConnectionReset on the first read instead of initially returning normal EOF. When a FIN frame carries a payload, deliver the payload before reporting EOF.

AI Assistance Disclosure

Tools used (required — write none if no AI was used): OpenAI Codex

Attestation (required):

  • I have read every line of this diff, understand what it does, and can explain it in review.

Notes & open questions

The protobuf schema permits a flag and payload in the same frame. The implementation therefore delays handling a payload-carrying FIN until its payload has been consumed. RESET remains immediate and discards its payload.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@getong

getong commented Aug 3, 2026

Copy link
Copy Markdown

Besides, webrtc releases 0.20, can you also update webrtc in this pr?
https://crates.io/crates/webrtc

@racequite

Copy link
Copy Markdown
Author

Besides, webrtc releases 0.20, can you also update webrtc in this pr? https://crates.io/crates/webrtc

Thanks for the heads-up. I tried updating webrtc from 0.17 to 0.20 locally, but it does not appear to be a straightforward dependency bump.

Besides the removal of the pem feature, 0.20 replaces the old APIBuilder / RTCPeerConnection and detached PollDataChannel APIs with the new PeerConnectionBuilder and event-based DataChannel API.

More importantly, the current libp2p WebRTC Direct transport relies on a custom UDP mux through SettingEngine::set_udp_network, while that support appears to be unavailable / still TODO in 0.20.

Would you prefer to keep this bug-fix PR focused and track the 0.20 migration separately? If the migration should be done in this PR, could you point me to the intended replacement for the custom UDP mux and detached data-channel APIs?

@getong

getong commented Aug 3, 2026

Copy link
Copy Markdown

https://github.com/getong/rust-libp2p/tree/webrtc-0.20
You can fork this branch , and copy some files from it. I am nor webrtc professional, I made it with codex.

@racequite

Copy link
Copy Markdown
Author

https://github.com/getong/rust-libp2p/tree/webrtc-0.20 You can fork this branch , and copy some files from it. I am nor webrtc professional, I made it with codex.

I think you should submit a separate PR to do this.

@getong

getong commented Aug 3, 2026

Copy link
Copy Markdown

I am not good at webrtc, you can take a review and copy it .

@racequite

Copy link
Copy Markdown
Author

I am not good at webrtc, you can take a review and copy it .

I'm just curious, are you the maintainer of this project or something else?

@getong

getong commented Aug 3, 2026

Copy link
Copy Markdown

no

@racequite

Copy link
Copy Markdown
Author

no

We'd better listen to the advice of the maintainers or members first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants