webrtc-rs / webrtc

A pure Rust implementation of WebRTC
https://webrtc.rs
Apache License 2.0
4.08k stars 363 forks source link

fix(data-channel): stuck closing in Chrome #480

Closed Marcel-G closed 1 year ago

Marcel-G commented 1 year ago

Resolves #468

Currently the library does not properly implement the shutdown procedure for data channels closed by Chrome. In order for the data channel to fully close, it must receive a OutgoingResetRequest.

This was an existing issue in pion which has since been resolved here https://github.com/pion/sctp/pull/238

Reproduction

Using the examples/data-channels Run the JS test for closing data channels: jsfiddle Note the RTCDataChannels never enter closed state.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 62.50% and project coverage change: -0.02% :warning:

Comparison is base (2f7df93) 61.80% compared to head (3b5f1fa) 61.79%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #480 +/- ## ========================================== - Coverage 61.80% 61.79% -0.02% ========================================== Files 530 530 Lines 48858 48860 +2 Branches 12324 12326 +2 ========================================== - Hits 30199 30194 -5 - Misses 9499 9502 +3 - Partials 9160 9164 +4 ``` | [Files Changed](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs) | Coverage Δ | | |---|---|---| | [examples/examples/data-channels/data-channels.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-ZXhhbXBsZXMvZXhhbXBsZXMvZGF0YS1jaGFubmVscy9kYXRhLWNoYW5uZWxzLnJz) | `0.00% <0.00%> (ø)` | | | [webrtc/src/data\_channel/data\_channel\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-d2VicnRjL3NyYy9kYXRhX2NoYW5uZWwvZGF0YV9jaGFubmVsX3Rlc3QucnM=) | `61.00% <0.00%> (ø)` | | | [interceptor/src/twcc/receiver/receiver\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-aW50ZXJjZXB0b3Ivc3JjL3R3Y2MvcmVjZWl2ZXIvcmVjZWl2ZXJfdGVzdC5ycw==) | `70.31% <100.00%> (ø)` | | | [media/src/audio/buffer/info.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-bWVkaWEvc3JjL2F1ZGlvL2J1ZmZlci9pbmZvLnJz) | `76.19% <100.00%> (-2.08%)` | :arrow_down: | | [sctp/src/queue/queue\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-c2N0cC9zcmMvcXVldWUvcXVldWVfdGVzdC5ycw==) | `65.00% <100.00%> (ø)` | | | [sctp/src/timer/timer\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-c2N0cC9zcmMvdGltZXIvdGltZXJfdGVzdC5ycw==) | `60.26% <100.00%> (ø)` | | | [sdp/src/direction/direction\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-c2RwL3NyYy9kaXJlY3Rpb24vZGlyZWN0aW9uX3Rlc3QucnM=) | `87.50% <100.00%> (ø)` | | | [sdp/src/extmap/extmap\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-c2RwL3NyYy9leHRtYXAvZXh0bWFwX3Rlc3QucnM=) | `77.77% <100.00%> (ø)` | | | [util/src/vnet/router/router\_test.rs](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs#diff-dXRpbC9zcmMvdm5ldC9yb3V0ZXIvcm91dGVyX3Rlc3QucnM=) | `60.70% <100.00%> (-0.19%)` | :arrow_down: | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/webrtc-rs/webrtc/pull/480/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webrtc-rs)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Marcel-G commented 1 year ago

@mxinden this remaining issues something you can help with?

rainliu commented 1 year ago

should we port the fix from https://github.com/webrtc-rs/sctp-proto/pull/6 to here?

Marcel-G commented 1 year ago

@rainliu yes, works like a charm!

rainliu commented 1 year ago

Great. Could you please fix format/clippy warning?

rainliu commented 1 year ago

Still clippy errors.Make sure you are using same or above cargo version in the GitHub actions.