fix(engine): TCP partial-read frame desync + per-tick inbound drain (T-1045)
FrameAccumulator state machine in framing.rs holds partial prefix/payload bytes across non-blocking receive() calls — a frame split across TCP segments no longer desyncs the stream (read_exact previously discarded partially-consumed bytes on WouldBlock). receive_bridge_inputs now drains all ready frames per tick (capped) instead of exactly one, covering input batches + atlas requests in the same window. Review hardening: EOF mid-frame escalates to Disconnected like clean EOF (peer died with a truncated stream) instead of logging an Io error every tick. Tests: frame split inside prefix / inside payload, multi-frame drain, mid-frame-EOF disconnect. Corrupt-stream escalation tracked as T-1072. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+80
-56
@@ -89,8 +89,11 @@ pub trait SimBridge: Send + Sync {
|
||||
/// Send an observer snapshot to the client
|
||||
fn send_snapshot(&self, snapshot: &ObserverSnapshot) -> Result<(), BridgeError>;
|
||||
|
||||
/// Receive one inbound message, or `None` if no frame is ready this tick.
|
||||
/// Receive one inbound message, or `None` if no complete frame is ready.
|
||||
/// The single client→server stream is demuxed by frame shape (D-225).
|
||||
/// `receive_bridge_inputs` loops this until `None` (T-1045), so the
|
||||
/// transport behind `BridgeResource` must not block when no frame is
|
||||
/// buffered (TcpBridge is non-blocking; LocalBridge blocks — test-only).
|
||||
fn receive(&self) -> Result<Option<Inbound>, BridgeError>;
|
||||
|
||||
/// Send an atlas layer-stream response to the client (#969, D-225).
|
||||
@@ -144,7 +147,15 @@ pub enum HandshakeState {
|
||||
Complete,
|
||||
}
|
||||
|
||||
/// Per-tick cap on drained inbound frames (T-1045) — a safety valve so a
|
||||
/// client flooding the stream cannot starve the simulation tick. Generous:
|
||||
/// normal traffic is one input batch plus the occasional atlas request.
|
||||
const MAX_INBOUND_FRAMES_PER_TICK: usize = 64;
|
||||
|
||||
/// Receive inputs from bridge and push to InputQueue.
|
||||
/// Drains every complete frame buffered this tick (T-1045) — a single
|
||||
/// receive() per tick would backlog mixed input/atlas traffic at one frame
|
||||
/// per 50 ms. Relies on receive() being non-blocking (Ok(None) = no frame).
|
||||
/// Protocol errors (malformed input) are recoverable: the frame is skipped
|
||||
/// and a SimError is pushed to the SimErrorBuffer for client reporting (#85).
|
||||
pub fn receive_bridge_inputs(
|
||||
@@ -159,64 +170,77 @@ pub fn receive_bridge_inputs(
|
||||
let Some(bridge) = bridge else { return };
|
||||
let current_tick = time.as_ref().map(|t| t.tick).unwrap_or(0);
|
||||
|
||||
match bridge.receive() {
|
||||
Ok(Some(Inbound::Inputs(inputs))) => {
|
||||
if !inputs.is_empty() && *handshake == HandshakeState::Pending {
|
||||
tracing::warn!(
|
||||
"Received {} input(s) before handshake completed — processing anyway (forward-compatible)",
|
||||
inputs.len()
|
||||
);
|
||||
for _ in 0..MAX_INBOUND_FRAMES_PER_TICK {
|
||||
match bridge.receive() {
|
||||
Ok(Some(Inbound::Inputs(inputs))) => {
|
||||
if !inputs.is_empty() && *handshake == HandshakeState::Pending {
|
||||
tracing::warn!(
|
||||
"Received {} input(s) before handshake completed — processing anyway (forward-compatible)",
|
||||
inputs.len()
|
||||
);
|
||||
}
|
||||
for input in &inputs {
|
||||
tracing::trace!(
|
||||
"Received input: tick={} action={:?}",
|
||||
input.tick,
|
||||
input.action
|
||||
);
|
||||
}
|
||||
for input in inputs {
|
||||
input_queue.push(input);
|
||||
}
|
||||
}
|
||||
for input in &inputs {
|
||||
tracing::trace!(
|
||||
"Received input: tick={} action={:?}",
|
||||
input.tick,
|
||||
input.action
|
||||
);
|
||||
Ok(Some(Inbound::AtlasRequest(req))) => {
|
||||
atlas_requests.0.push(req);
|
||||
}
|
||||
for input in inputs {
|
||||
input_queue.push(input);
|
||||
// No complete frame ready — the backlog is drained.
|
||||
Ok(None) => break,
|
||||
Err(BridgeError::Disconnected) => {
|
||||
tracing::info!("Client disconnected, shutting down");
|
||||
running.0 = false;
|
||||
break;
|
||||
}
|
||||
Err(BridgeError::Io(ref e))
|
||||
if e.kind() == std::io::ErrorKind::BrokenPipe
|
||||
|| e.kind() == std::io::ErrorKind::ConnectionReset =>
|
||||
{
|
||||
tracing::info!("Pipe broken, shutting down cleanly");
|
||||
running.0 = false;
|
||||
break;
|
||||
}
|
||||
Err(BridgeError::MutexPoisoned(ref msg)) => {
|
||||
tracing::error!("Bridge mutex poisoned: {}. Shutting down.", msg);
|
||||
running.0 = false;
|
||||
break;
|
||||
}
|
||||
Err(BridgeError::DeserializationWithDump(ref msg)) => {
|
||||
// Recoverable: skip this frame's input, report to client (#85),
|
||||
// keep draining — the frame was consumed, later ones may be fine.
|
||||
tracing::error!("Skipping malformed input frame: {}", msg);
|
||||
error_buffer.push(SimError {
|
||||
kind: SimErrorKind::ProtocolError,
|
||||
message: format!("Malformed input frame: {}", msg),
|
||||
tick: current_tick,
|
||||
});
|
||||
}
|
||||
Err(ref e @ BridgeError::Deserialization(_)) => {
|
||||
// Recoverable deserialization error without dump
|
||||
tracing::error!("Skipping malformed input: {}", e);
|
||||
error_buffer.push(SimError {
|
||||
kind: SimErrorKind::ProtocolError,
|
||||
message: format!("Deserialization error: {}", e),
|
||||
tick: current_tick,
|
||||
});
|
||||
}
|
||||
Err(e) => {
|
||||
// Unknown error: log once per tick instead of hammering a
|
||||
// persistently failing stream within one tick. A permanently
|
||||
// corrupt stream (e.g. the oversized-prefix poison state)
|
||||
// therefore logs every tick without escalation — follow-up
|
||||
// ticket covers shutdown-after-N-consecutive-errors.
|
||||
tracing::error!("Bridge receive error: {}", e);
|
||||
break;
|
||||
}
|
||||
}
|
||||
Ok(Some(Inbound::AtlasRequest(req))) => {
|
||||
atlas_requests.0.push(req);
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(BridgeError::Disconnected) => {
|
||||
tracing::info!("Client disconnected, shutting down");
|
||||
running.0 = false;
|
||||
}
|
||||
Err(BridgeError::Io(ref e))
|
||||
if e.kind() == std::io::ErrorKind::BrokenPipe
|
||||
|| e.kind() == std::io::ErrorKind::ConnectionReset =>
|
||||
{
|
||||
tracing::info!("Pipe broken, shutting down cleanly");
|
||||
running.0 = false;
|
||||
}
|
||||
Err(BridgeError::MutexPoisoned(ref msg)) => {
|
||||
tracing::error!("Bridge mutex poisoned: {}. Shutting down.", msg);
|
||||
running.0 = false;
|
||||
}
|
||||
Err(BridgeError::DeserializationWithDump(ref msg)) => {
|
||||
// Recoverable: skip this frame's input, report to client (#85)
|
||||
tracing::error!("Skipping malformed input frame: {}", msg);
|
||||
error_buffer.push(SimError {
|
||||
kind: SimErrorKind::ProtocolError,
|
||||
message: format!("Malformed input frame: {}", msg),
|
||||
tick: current_tick,
|
||||
});
|
||||
}
|
||||
Err(ref e @ BridgeError::Deserialization(_)) => {
|
||||
// Recoverable deserialization error without dump
|
||||
tracing::error!("Skipping malformed input: {}", e);
|
||||
error_buffer.push(SimError {
|
||||
kind: SimErrorKind::ProtocolError,
|
||||
message: format!("Deserialization error: {}", e),
|
||||
tick: current_tick,
|
||||
});
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::error!("Bridge receive error: {}", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user