frame/io: Introduce Poisoned state for WriteState#211
Merged
jxs merged 1 commit intolibp2p:masterfrom Oct 23, 2025
Merged
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
jxs
approved these changes
Oct 23, 2025
Member
jxs
left a comment
jxs
left a comment
There was a problem hiding this comment.
Hi Alexandru and thanks for this follow up. LGTM.
Do you need a release with this?
Contributor
Author
|
Hey @jxs thanks a lot for the quick review!
Yep, would love to have this in a release 🙏 |
lexnv
added a commit
to paritytech/litep2p
that referenced
this pull request
Jan 20, 2026
…plementation (#518) The PR fixes a `AsyncWrite::poll_write` implementation of the crypto/noise sockets that causes panics in rust-yamux and leads to unnecessary connection closure and instability: - T0: poll_write is called with buffer of len 512 bytes - the implementation encrypt data and buffers it - because the io socket is not ready, poll pending is returned. - T1: tokio_tungstenite (or other component) decides that a new payload must be sent (usually a PONG frame) of len 12 bytes - because the inner buffers contain the previous message, upon flushing the size of the older message is returned (ie 512) - rust-yamux uses the 512 bytes to index a buffer of 12 bytes (as expected by the second poll_write with buffer 12) - This caused frequent panics on the websocket implementation, which is currently addressed as abrupt connection termination This PR fixes the `AsyncWrite` contract violation by effectively decoupling the encryption from the writing steps. - the inner buffers are drained as much as possible (until the socket returns poll pending) - The provided message is encrypted into the inner buffer if it has capacity and the number of bytes is returned immediately - a subsequent poll_write or poll_flush or poll_close will propagate the encrypted buffered data to the underlying socket Previous fixes: - libp2p/rust-yamux#202 - libp2p/rust-yamux#211 The fixes are still needed since the tokio-tungstenite (websocket crate) was not properly scoped and may exhibit similar behavior. cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
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
2 participants
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.
The Sink implementation of the yamux-frame could panic when the number of bytes from a write operation exceeds the header size. This edge case was fixed in:
However,
poll_flushandpoll_closewould still call intopoll_ready. In these cases, the frame/io component would still panic since theoffsetis bigger than the actual header. To mitigate this, the PR introduces aPoisonedstate, and further calls topoll_readywould result in an immediate error.// cc @jxs @elenaf9