Skip to content

Commit 300673c

Browse files
authored
yamux/control: Ensure poll next inbound is not called after errors (#445)
We have recently fixed a spurious issue that caused the rust-yamux component to panic. Litep2p transitioned the internal state of the connection back to Idle. This causes litep2p to poll the component again. Then rust-yamux would use the same buffer offset that caused the panic in the first place. Instead, this PR ensures that litep2p never polls the connection once yamux encounters an error. This is aligned with the behavior of transport layers that close the connection on the first yamux error. Part of: paritytech/polkadot-sdk#9169 cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
1 parent 5a09ff9 commit 300673c

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ x25519-dalek = "2.0.1"
5050
x509-parser = "0.17.0"
5151
yasna = "0.5.0"
5252
zeroize = "1.8.1"
53-
yamux = "0.13.5"
53+
yamux = "0.13.7"
5454

5555
# Websocket related dependencies.
5656
tokio-tungstenite = { version = "0.27.0", features = ["rustls-tls-native-roots", "url"], optional = true }

src/yamux/control.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ use std::{
1919
task::{Context, Poll},
2020
};
2121

22+
const LOG_TARGET: &str = "litep2p::yamux::control";
23+
2224
/// A Yamux [`Connection`] controller.
2325
///
2426
/// This presents an alternative API for using a yamux [`Connection`].
@@ -81,7 +83,25 @@ where
8183
State::Idle(mut connection) => {
8284
match connection.poll_next_inbound(cx) {
8385
Poll::Ready(maybe_stream) => {
84-
self.state = State::Idle(connection);
86+
// Transport layers will close the connection on the first
87+
// substream error. The `connection.poll_next_inbound` should
88+
// not be called again after returning an error. Instead, we
89+
// must close the connection gracefully.
90+
match maybe_stream.as_ref() {
91+
Some(Err(error)) => {
92+
tracing::debug!(target: LOG_TARGET, ?error, "Inbound stream error, closing connection");
93+
94+
self.state = State::Closing {
95+
reply: None,
96+
inner: Closing::DrainingControlCommands { connection },
97+
};
98+
}
99+
other => {
100+
tracing::debug!(target: LOG_TARGET, ?other, "Inbound stream reset state to idle");
101+
self.state = State::Idle(connection)
102+
}
103+
}
104+
85105
return Poll::Ready(maybe_stream);
86106
}
87107
Poll::Pending => {}
@@ -191,7 +211,7 @@ where
191211
return Poll::Pending;
192212
}
193213
},
194-
State::Poisoned => unreachable!(),
214+
State::Poisoned => return Poll::Pending,
195215
}
196216
}
197217
}

0 commit comments

Comments
 (0)