Skip to content

A panicking binary-stage connection worker stops the accept loop #1473

Description

@danielwinterw

Raised out of review on #1409, where CodeRabbit flagged it against code that predates the PR. Filing separately rather than widening that diff.

ConnectionWorkers::reap_finished in crates/skippy-server/src/binary_transport/binary_messaging.rs turns a panicked worker into an error:

if self.0[index].task.is_finished() {
    let worker = self.0.swap_remove(index);
    if worker.task.join().is_err() {
        bail!("binary stage connection worker panicked");
    }
}

The accept loop calls connection_workers.reap_finished()?, so a panic in any one connection's worker thread propagates out of the loop and takes down the listener. Every other live connection then goes through shutdown, and the stage stops accepting new ones — a single malformed request or unexpected state on one connection can end serving for the whole process.

Containing it per-connection would match how the iteration scheduler already handles worker panics (SchedulerWorker::run catches unwind and fails just that worker's requests): log the panic, fail that connection, and keep accepting.

Introduced with the connection-worker tracking in #1420. Not triggered by anything in the run-ahead stack; noticed while reviewing it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions