transport/udp: detach peer-drain worker thread on Drop to avoid runtime-driver deadlock

PeerRecvDrain::drop previously called std::thread::join on the worker
thread synchronously. The worker uses packet_tx.blocking_send on a
tokio mpsc Sender, which internally parks the worker via
tokio::block_on on the same current_thread runtime that drives
rx_loop. Calling join from inside remove_active_peer (which runs on
the runtime thread, the runtime's sole driver) created a circular
wait:

  - rx_loop blocks in libc futex via Thread::join
  - the worker being joined cannot observe the stop flag because the
    runtime that polls it is the very thread now blocked joining it
  - all other PeerRecvDrain workers park on the same runtime via
    block_on, so a single peer's removal wedges every worker on the
    daemon

The /proc snapshot from a production wedge showed exactly this
shape: 107 of 108 threads in futex_do_wait, 101 of them named
fips-peer-drain. fipsctl became unresponsive (EAGAIN on control
socket), SIGTERM was ignored, and Docker SIGKILLed the container
after the 10 s grace period. Two confirmed wedges on the public
test deployment (52 min and 23 min uptime), plus a third on the
admission-gate-Msg2-silent-drop build at 2 min 21 sec — all ending
with the identical "Peer removed and state cleaned up
tree_changed=false" final log line preceding total silence.

Fix: detach the std::thread instead of joining. The stop flag plus
self-pipe write already signal the worker to exit; the worker's
kernel-level libc::poll inside the drain loop sees the wake, checks
the flag, exits, and the OS reclaims the thread state independently
of the JoinHandle being dropped.

The trigger was statistically amplified by aggressive multi-npub-
from-one-NAT peer reconnect patterns at the moment of the 30 s
link-dead-timeout peer-removal, but not bounded to them. Any
peer whose disconnect happens with the per-peer drain worker
parked in block_on can fire the bug. The admission-gate work
that landed earlier in this branch line compressed more handshake
work per rx_loop tick, increasing the rate at which workers are
parked in block_on and so reducing time-to-wedge — but the
underlying bug pre-dated the admission gate and pre-dated this
fix branch.

The deployed wedged daemon is mitigated operationally by blocking
the trigger IP at the host firewall; this commit removes the bug
class entirely.
This commit is contained in:
Johnathan Corgan
2026-05-30 04:24:03 +00:00
parent 5987b54730
commit 2d0e8de8c8
2 changed files with 33 additions and 5 deletions
+19
View File
@@ -193,6 +193,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
accounting change. Operators with mixed outbound + inbound
deployments no longer see legitimate inbound peers rejected once
outbound connections fill the pool past the configured cap.
- `PeerRecvDrain::drop` no longer calls `std::thread::join` on the
worker thread. The drain worker uses `packet_tx.blocking_send(...)`
on a tokio mpsc Sender, which internally parks the worker in
`tokio::block_on` on the same `current_thread` runtime that drives
`rx_loop`. Joining synchronously from inside `remove_active_peer`
(which runs on the runtime thread, the runtime's sole driver)
produced a circular wait: rx_loop blocked in libc futex via
`Thread::join`, the worker unable to observe the stop flag because
the runtime that polls it is the very thread now blocked joining
it, and all other peer-drain workers parked on the same runtime
via `block_on`. Full daemon wedge, fipsctl unresponsive, SIGTERM
ignored. Trigger was peer-removal via the 30-s link-dead-timeout
cleanup path with any in-flight worker, with statistical likelihood
amplified by aggressive multi-npub-from-one-NAT reconnect patterns
but not bounded to them. Fix: detach the std::thread (drop the
`JoinHandle` without joining); the stop flag + self-pipe write
already signal the worker to exit; the kernel-level `libc::poll()`
inside the drain loop sees the wake, checks the flag, exits, and
the OS reclaims the thread state independently.
- Outbound connection initiation now honors the `node.limits.max_peers`
cap that was previously only checked on inbound msg1 admission. Four
paths gated: auto-reconnect retries (`process_pending_retries`),