Skip to content

Commit 8126bcf

Browse files
committed
Read the router's ready byte before reuse
A recycled connection is not free until the peer's reader lets it go, and rumors readers can be lazy. The pool handed connections out as soon as the writer finished, so a stream could sit undelivered behind the previous stream's consumer. Mutual gossip sessions wait on each other's streams, and the extra dependency closed a wait cycle. The CI conformance suite deadlocked there. The router now writes one byte on a recovered connection when it starts reading for the next header. Recycle reads that byte in a spawned task and clears the dirty bit only once it arrives, so the pool never hands out a connection the peer still holds. max_slots grows to cover claims parked on the byte.
1 parent f969073 commit 8126bcf

3 files changed

Lines changed: 34 additions & 35 deletions

File tree

Cargo.lock

Lines changed: 16 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ rand = "0.8"
4242
rand_core = "0.6"
4343
reqwest = { version = "0.13", features = ["json", "rustls"] }
4444
rlimit = "0.10"
45-
rumors = { git = "https://github.com/oxidecomputer/rumors", rev = "4be4b8308b38d6961e7ccaf613394e7d5bbf3f6d" }
45+
rumors = { git = "https://github.com/oxidecomputer/rumors", rev = "b2675dbef99b784efb817db01920ee59dac708c3" }
4646
rustix = { version = "1", features = ["fs", "process", "pty", "termios"] }
4747
rustls = "0.23"
4848
rustyline = { version = "17", features = ["termios"] }

server/src/link.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use rumors::link::routed::{Config, Dial, Endpoint, Incoming, Listen, RoutedLink}
3939
use slog::{Logger, o, warn};
4040
use sprockets_tls::keys::SprocketsConfig;
4141
use sprockets_tls::{Client, Server};
42-
use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt as _, ReadBuf};
42+
use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _, ReadBuf};
4343
use tokio::net::TcpStream;
4444
use tokio::runtime::Handle;
4545
use tokio::sync::mpsc;
@@ -65,9 +65,11 @@ const PENDING_HEADERS: usize = STREAM_COUNT * 32;
6565
const ACCEPT_RETRY: Duration = Duration::from_millis(100);
6666

6767
/// Connections per peer. The link contract's worst case is one control
68-
/// stream plus a full complement for each of a pair's two links. Slots
69-
/// are created on demand, so the cap is free when idle.
70-
const MAX_SLOTS: usize = 2 * STREAM_COUNT + 1;
68+
/// stream plus a full complement for each of a pair's two links, and
69+
/// claims parked on the ready byte can briefly overlap the next
70+
/// session's opens. Slots are created on demand, so the cap is free
71+
/// when idle.
72+
const MAX_SLOTS: usize = 3 * STREAM_COUNT + 1;
7173

7274
/// Idle connections kept warm per peer. Taking one stream leaves a
7375
/// spare, so qorb fires no refill and reuses the recycled connection
@@ -347,11 +349,17 @@ impl Dial for SprocketsDial {
347349
}
348350

349351
fn recycle(&self, _peer: &SocketAddr, mut conn: SprocketsConn) {
350-
// The stream completed cleanly, so the connection may be
351-
// reused: clear the bit and let the drop return the claim.
352-
if let Conn::Pooled(handle) = &mut conn.0 {
353-
handle.dirty = false;
354-
}
352+
// Reusable only once the peer's router says it is ready: read
353+
// that byte off the session's task, then let the drop return
354+
// the claim clean.
355+
spawn(async move {
356+
let mut ready = [0u8; 1];
357+
if conn.read_exact(&mut ready).await.is_ok()
358+
&& let Conn::Pooled(handle) = &mut conn.0
359+
{
360+
handle.dirty = false;
361+
}
362+
});
355363
}
356364
}
357365

0 commit comments

Comments
 (0)