Skip to content

[runtime/network] tokio dial() has no connect timeout — a black-holed SYN holds a peer reservation for the full OS connect-timeout, and persistent records keep re-dialing into it #4175

Description

@marthendalnunes

Summary

The tokio runtime's outbound dial has no connect timeout. Network::dial awaits a bare TcpStream::connect(socket) with nothing bounding it (runtime/src/network/tokio.rs:328-335):

async fn dial(
    &self,
    socket: SocketAddr,
) -> Result<(crate::SinkOf<Self>, crate::StreamOf<Self>), crate::Error> {
    // Create a new TCP stream
    let stream = TcpStream::connect(socket)
        .await
        .map_err(|_| Error::ConnectionFailed)?;
    // ...
}

If the peer's address is reachable at the routing layer but the SYN is silently dropped — a black hole rather than a RST (e.g. a stale conntrack entry, an ACL/firewall that drops instead of rejects, or a peer that moved after a reschedule while the old path still routes) — this future does not resolve until the OS gives up. On Linux that is tcp_syn_retries retransmissions, ~127 s (≈2 min) by default, before ETIMEDOUT. A RST (connection refused) fails fast; the black-holed SYN is the pathological case.

The runtime's existing read/write timeouts do not cover this: they wrap send/recv on an already-established stream. Sink::send bounds the write with timeout(write_timeout, send) (tokio.rs:79) and Stream::recv bounds the read with timeout(read_timeout, recv) (tokio.rs:127). Config exposes with_read_timeout / with_write_timeout (tokio.rs:216-228, 250-258) and their doc comments explicitly scope them to Stream::recv / Sink::send — there is no analogous connect bound. Grepping connect_timeout / dial_timeout across runtime/ and p2p/ returns nothing: no connect timeout config exists on any code path today (checked deterministic, iouring, metered, audited, and tokio dial impls).

Why this is more than a single 2-minute stall

In authenticated/discovery, the dialer holds the peer's tracker reservation for the entire duration of the connect future, and the reservation is the only thing gating re-dial of that peer:

  1. The dialer spawns a child task that owns the Reservation and calls context.dial(address).await inside it (p2p/src/authenticated/discovery/actors/dialer.rs:101-140, dial at :118). On every failure path the closure returns and drops the reservation; on success it is handed to the supervisor (:138). So the reservation is held for as long as the connect hangs.
  2. Reservation release is Drop-only (.../tracker/reservation.rs:35-43) — there is no timer. The tracker actor imposes no independent deadline on a held reservation (the only sleeps in .../tracker/actor.rs are in its #[cfg(test)] module).
  3. While the record is Status::Reserved, Record::reserve returns ReserveResult::Unavailable (.../tracker/record.rs:185), so the peer cannot be re-reserved or re-dialed for the whole hang.

So a single black-holed SYN pins that peer's reservation for the OS connect-timeout (~2 min), during which the node makes no progress toward (re)connecting it — even though every discovery timer is far shorter. Under discovery::Config::local (p2p/src/authenticated/discovery/config.rs): peer_connection_cooldown 1 s (:203), dial_frequency 500 ms (:207), gossip_bit_vec_frequency 5 s (:211); under recommended: 60 s / 1 s / 50 s (:160, :164, :168). All are well below ~2 min.

It also does not self-clear after one cycle for a persistent peer. reserve sets next_reservable_at = now + interval at reservation time (record.rs:193), not at release — so once the ~2-min connect finally errors and the reservation drops, the peer is already past its cooldown and immediately re-reservable. And persistent records (bootstrappers, record.rs:106-116, persistent: true) are never evicted or backed off on dial failure: deletable() is false for persistent (:333-338); eligible() / is_outbound_target() stay true regardless of set membership or failure count (:171-173, :346-353); and dial_failure on a Bootstrapper address is a no-op — it only mutates the Discovered fail counter (:214-220). So the node re-dials the same black-holed address, re-enters the same ~2-min hang, and repeats. A reconnect that should take seconds can instead take many minutes, and for a bootstrapper it persists until the black hole clears at the network layer or the process is restarted.

This is a gap in the recovery model, not a delegated responsibility

Unreachable-peer recovery is clearly within discovery's intended scope — the suite tests it directly. test_dns_mixed_ips_connectivity (p2p/src/authenticated/discovery/mod.rs:1477) resolves a hostname to a set where most IPs are unreachable and asserts that a node still connects because "the dialer randomly picks an IP, so eventually it should pick the reachable one." The wrong-address test (mod.rs:1921) points a restarted peer at an unreachable 10.255.255.1 and recovers by having the other peer dial in — the design intent being "if A can't reach B, B dials A."

Both of those recovery paths assume connect failures are fast. 10.255.255.1 (an unrouteable RFC5737-style address) returns EHOSTUNREACH/ENETUNREACH immediately, so the reservation frees at once and the random-IP retry or the reverse-dial proceeds within the discovery timers. The deterministic runtime used by these tests likewise models connect failure as immediate. A black-holed SYN is the case neither path covers: the connect neither succeeds nor fails-fast — it hangs for the full OS connect-timeout, holding the reservation the whole time (per the section above). So the reverse-dial recovery the design relies on is delayed by ~2 min per cycle, and for a persistent record it re-hangs indefinitely.

In other words, this isn't a caller responsibility that discovery deliberately delegates — the dialer doesn't wrap context.dial() in any timeout or select either (dialer.rs:118), and there is no doc anywhere in runtime/ or p2p/ scoping connect-timeout to the caller. It's a timing assumption baked into the tested recovery model that a silent SYN-drop violates.

Suggested fix

  • Bound the connect. Wrap dial's TcpStream::connect in a configurable connect_timeout via the same runtime timeout primitive already used for read/write (tokio::time::timeout, mirroring tokio.rs:79/:127), returning Error::Timeout on expiry. Add connect_timeout to network::tokio::Config alongside read_timeout / write_timeout, and thread an equivalent bound through the other Network::dial impls so behavior is uniform across runtimes. A default in the tens of seconds would already cut the worst case from ~2 min to a bounded, configurable value.
  • Defense in depth on established connections. Consider enabling SO_KEEPALIVE / TCP keepalive so a black hole that develops after a connection is established (a mid-stream path drop) is detected and torn down without waiting on application-level read timeouts alone.
  • Bound the reservation independently of the dial future. Even with a connect timeout, tying the reservation lifetime solely to the dial future couples "how long a peer is un-redialable" to "how long a single connect may take." A cap on how long a reservation may sit in Status::Reserved (releasing and logging on expiry) would keep a single wedged dial from starving that peer's re-dial regardless of the transport-level timeout.

Minimal repro sketch

Point a dialer at an address that accepts routing but never completes the handshake — either a firewall rule that DROPs (not REJECTs) SYNs to a port, or a listener that accept()s and then immediately drops the socket without responding. Observe that Network::dial does not return for ~tcp_syn_retries worth of time (~2 min on default Linux) for the DROP case, and that the corresponding tracker record stays Status::Reserved — un-redialable — for that whole window. A deterministic-runtime variant can assert that dial outlives every configured discovery interval when the peer never completes connection setup.

Happy to contribute the connect-timeout wiring (+ the reservation cap, if wanted) and a regression test as a PR if the direction sounds right.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions