Skip to content

Commit e16b2d1

Browse files
joelteplyclaude
andauthored
fix(lan): the stable port survives the restart handover — a lost race no longer demotes a node to an address nobody has (#1301) (#1369)
`stable_lan_port` was correct. Its unit test asserted "same identity must derive the same port across restarts" and passed. The node churned its port on every restart anyway, because of how the bind site handled failure: Err(_preferred_taken) => adapter.listen((UNSPECIFIED, 0)).await Measured on the Windows node 2026-08-15, immediately after `airc update`: peer_id e85a5bb3-74f0-4325-87df-7d5f27637063 -> stable port 61539 daemon advertising -> 64463 61539 at that moment -> BINDABLE. Free, nothing listening, not in any Windows excluded range. The new daemon raced the outgoing one during the update handover, lost by milliseconds, took an ephemeral port, and then held it for the entire process lifetime — after the stable port freed. Every endpoint peers had cached for this node pointed at a dead port. M5 observed the other half independently and without knowing the cause: "your airc INBOUND is unreachable, that is the whole 'everyone waiting on everyone' mystery." Three defects in that one expression: 1. NO RETRY. The contended moment IS the restart handover — the single most common moment this code runs. One attempt loses it. 2. SILENT. `_preferred_taken` discards the reason, so a node that has just become unreachable at every address its peers know reports nothing. That is the masking fallback this repo denies at the clippy gate, written longhand. 3. NO RECOVERY. Once ephemeral, ephemeral for the whole process. Now: retry the identity port briefly (5 x 200ms — bounded, because a genuinely occupied port must surface fast rather than stall startup), and if it still cannot be had, take an ephemeral one but WARN loudly naming the consequence. "Reachable at an address nobody has" is precisely the failure that reads as a quiet room rather than a broken wire, so it must never again be silent. The regression test reproduces the handover: hold the stable port, release it mid-window, assert the bind still lands on the stable port. Positive control run rather than assumed — with the retry reduced to one attempt (the old behaviour) it FAILS on an ephemeral port (64596 vs 64185); restored, it passes. This is the mechanism behind "routes decay to zero after every update", which is what made "always run the latest binary" and "stay connected" fight each other on this node. Verified: cargo test -p airc-lib --lib lan:: — 8 passed, 0 failed. Claude-Session: https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2b5bdac commit e16b2d1

1 file changed

Lines changed: 137 additions & 10 deletions

File tree

crates/airc-lib/src/lan.rs

Lines changed: 137 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,9 @@ impl Airc {
151151
// the root of the cross-machine auto-connect churn (#8). Fall back to
152152
// an OS-assigned port only if the preferred one is already taken.
153153
let preferred = stable_lan_port(self.inner.identity.peer_id);
154-
let actual = match adapter
155-
.listen(SocketAddr::from((Ipv4Addr::UNSPECIFIED, preferred)))
156-
.await
157-
{
158-
Ok(addr) => addr,
159-
Err(_preferred_taken) => adapter
160-
.listen(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0)))
161-
.await
162-
.map_err(|error| AircError::Transport(error.to_string()))?,
163-
};
154+
let actual = self
155+
.bind_preferred_or_ephemeral(&adapter, preferred)
156+
.await?;
164157
self.ensure_lan_subscriber().await?;
165158
self.upsert_transport_health(TransportHealthSample::healthy_direct(TransportKind::LanTcp))?;
166159
let port = actual.port();
@@ -194,6 +187,85 @@ impl Airc {
194187
Ok(advertised)
195188
}
196189

190+
/// Bind the identity-derived port, RETRYING across a restart handover,
191+
/// and never demote to an ephemeral port in silence.
192+
///
193+
/// The stable port is the whole mechanism behind peers' cached endpoints
194+
/// surviving a restart (#8). It had a correct derivation, a passing unit
195+
/// test asserting "same identity must derive the same port across
196+
/// restarts", and a bind site that tried it first — and the node still
197+
/// churned its port on every restart, because of the two lines that
198+
/// handled failure:
199+
///
200+
/// ```ignore
201+
/// Err(_preferred_taken) => adapter.listen((UNSPECIFIED, 0)).await
202+
/// ```
203+
///
204+
/// Measured on the Windows node 2026-08-15, immediately after an
205+
/// `airc update`: peer_id `e85a5bb3-…` derives port 61539, the daemon
206+
/// was advertising 64463, and 61539 was BINDABLE at that moment — free,
207+
/// not excluded, nothing listening. The new daemon had raced the
208+
/// outgoing one during the update handover, lost, taken an ephemeral
209+
/// port, and then kept it for the whole process lifetime even after the
210+
/// stable port freed milliseconds later.
211+
///
212+
/// Three defects in one expression, all of which this fixes:
213+
///
214+
/// 1. NO RETRY. The contended window is the restart handover itself —
215+
/// the single most common moment this code runs. One attempt loses it.
216+
/// 2. SILENT. `_preferred_taken` discards the reason, so a node that has
217+
/// just become unreachable at every address its peers have cached
218+
/// reports nothing at all. That is the masking fallback this repo
219+
/// denies at the clippy gate, written in longhand.
220+
/// 3. NO RECOVERY. Once ephemeral, always ephemeral for that process.
221+
///
222+
/// This retries briefly, and if it still cannot get the stable port it
223+
/// takes an ephemeral one — but says so LOUDLY and names the
224+
/// consequence, because "reachable at an address nobody has" is exactly
225+
/// the failure that reads as a quiet room rather than a broken wire.
226+
async fn bind_preferred_or_ephemeral(
227+
&self,
228+
adapter: &LanTcpAdapter,
229+
preferred: u16,
230+
) -> Result<SocketAddr, AircError> {
231+
// Short and bounded: this covers an outgoing daemon releasing its
232+
// listener, which is a sub-second handover. It is deliberately not a
233+
// long wait — a genuinely occupied port must surface fast rather
234+
// than stall startup.
235+
const ATTEMPTS: u32 = 5;
236+
const RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(200);
237+
238+
let mut last_error = None;
239+
for attempt in 1..=ATTEMPTS {
240+
match adapter
241+
.listen(SocketAddr::from((Ipv4Addr::UNSPECIFIED, preferred)))
242+
.await
243+
{
244+
Ok(addr) => return Ok(addr),
245+
Err(error) => {
246+
last_error = Some(error.to_string());
247+
if attempt < ATTEMPTS {
248+
tokio::time::sleep(RETRY_DELAY).await;
249+
}
250+
}
251+
}
252+
}
253+
254+
let addr = adapter
255+
.listen(SocketAddr::from((Ipv4Addr::UNSPECIFIED, 0)))
256+
.await
257+
.map_err(|error| AircError::Transport(error.to_string()))?;
258+
259+
tracing::warn!(
260+
preferred_port = preferred,
261+
fallback_port = addr.port(),
262+
attempts = ATTEMPTS,
263+
last_error = last_error.as_deref().unwrap_or("unknown"),
264+
"LAN bind fell back to an EPHEMERAL port — this node is now unreachable at the identity-derived port every peer has cached for it, and will stay on the ephemeral port until restarted. Peers must rediscover it through the rendezvous before any frame can cross."
265+
);
266+
Ok(addr)
267+
}
268+
197269
/// Every socket address stored as ANOTHER peer's dialable endpoint
198270
/// (both trust stores, same merge scope as the dialer). Input to the
199271
/// advertise collision guard — we must never advertise an address the
@@ -466,6 +538,61 @@ mod tests {
466538
}
467539
}
468540

541+
/// what this catches: the RESTART HANDOVER RACE that silently demoted a
542+
/// node to an ephemeral port — and kept it there.
543+
///
544+
/// `stable_lan_port` was correct, and the sibling test above proved it
545+
/// derived the same port every time. The node churned its port anyway,
546+
/// because the bind site treated "preferred port busy" as a one-shot and
547+
/// fell through to `:0` while discarding the reason:
548+
///
549+
/// ```ignore
550+
/// Err(_preferred_taken) => adapter.listen((UNSPECIFIED, 0)).await
551+
/// ```
552+
///
553+
/// The contended moment is the restart handover itself — the outgoing
554+
/// daemon still holds the listener for a few hundred ms — which is the
555+
/// single most common moment this code runs. Measured on the Windows
556+
/// node 2026-08-15 right after an `airc update`: identity `e85a5bb3-…`
557+
/// derives 61539, the daemon advertised 64463, and 61539 was BINDABLE at
558+
/// that moment. It lost the race, took an ephemeral port, and held it for
559+
/// the process lifetime — so every endpoint peers had cached pointed at a
560+
/// dead port and inbound was structurally unreachable. M5 observed the
561+
/// other half independently: "your airc INBOUND is unreachable."
562+
///
563+
/// This occupies the stable port, releases it mid-handover, and asserts
564+
/// the bind still lands on the STABLE port. Against the old one-shot code
565+
/// it fails with an ephemeral port, which is the regression.
566+
#[tokio::test]
567+
async fn a_busy_stable_port_is_retried_across_the_restart_handover() {
568+
let (_dir, airc) = test_airc().await;
569+
let preferred = stable_lan_port(airc.inner.identity.peer_id);
570+
571+
// Stand in for the outgoing daemon still holding the listener.
572+
let outgoing =
573+
std::net::TcpListener::bind(SocketAddr::from((Ipv4Addr::UNSPECIFIED, preferred)))
574+
.expect("test must be able to hold the stable port to simulate the handover");
575+
576+
// Release it partway through the retry window, as a real handover does.
577+
tokio::spawn(async move {
578+
tokio::time::sleep(std::time::Duration::from_millis(400)).await;
579+
drop(outgoing);
580+
});
581+
582+
let advertised = airc
583+
.listen_lan_advertising(Some(Ipv4Addr::new(192, 168, 1, 50)), None)
584+
.await
585+
.expect("bind must succeed once the outgoing listener releases");
586+
587+
let addr = lan_addr(&advertised).expect("a LAN endpoint must be advertised");
588+
assert_eq!(
589+
addr.port(),
590+
preferred,
591+
"bind fell back to an ephemeral port ({}) instead of retrying the identity-derived port ({preferred}) across the handover — every endpoint peers have cached for this node now points at a dead port, and the node cannot tell that it is unreachable",
592+
addr.port()
593+
);
594+
}
595+
469596
async fn test_airc() -> (tempfile::TempDir, Airc) {
470597
let dir = tempdir().unwrap();
471598
let airc = Airc::open_with_wire_root_for_test(

0 commit comments

Comments
 (0)