rust-libp2p
rust-libp2p copied to clipboard
feat(webrtc): implement fin ack
Description
Implements FIN_ACK support for WebRTC streams to ensure reliable closure handshaking as specified in the WebRTC spec. This adds:
- FIN_ACK flag handling in state machine
- Timeout mechanism for FIN_ACK wait
- Proper state transitions for graceful closure
- https://github.com/libp2p/rust-libp2p/issues/4600
Notes & open questions
- Using
Instantfor deadline tracking instead of tokio timer to avoid extra dependencies - Timeout is set to 10 seconds - should this be configurable?
- State machine ensures FIN_ACK is only sent/processed in appropriate states
Change checklist
- [x] I have performed a self-review of my own code
- [ ] I have made corresponding changes to the documentation
- Added documentation for FIN_ACK related functions
- Updated state machine documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- Added tests for FIN_ACK state transitions
- Added tests for timeout behavior
- Added tests for edge cases
Thank you for your contribution @tesol2y090 and sorry for the delay here! Will try to give this a review in the next days.
This pull request has merge conflicts. Could you please resolve them @tesol2y090? 🙏
Regarding whether poll_close should wait for the FIN_ACK or not (see https://github.com/libp2p/rust-libp2p/pull/5687#discussion_r1898455099).
As already mentioned, the current implementation won't work when there are blocking calls to close().await, because without a concurrent call to poll_read the corresponding FIN_ACK will never be read and we'll always timeout.
If the read side is closed anyway, we can just read from within poll_close, parse the flags, and discard any other messages analogous to how poll_write is doing it.
If the read side isn't closed yet we have two options:
- Don't wait for the
FIN_ACK. But that would mean that we wouldn't have the guarantee that all data has been received. - Read from the stream and buffer the incoming messages up to a
BUFFER_MAX(I think we can use the existingread_bufferfor that?). If the max has been reached before theFIN_ACKwe just return same like in case of a timeout.
I think we should go with nr. 2, but I wonder if there are any better options that I didn't think of (cc @dariusc93 @jxs). What do you think @tesol2y090?