From 4a232b4e8600aeba33c4fb41f31a5923a3b33c18 Mon Sep 17 00:00:00 2001 From: Dr Luke Angel Date: Fri, 29 May 2026 08:11:20 -0400 Subject: [PATCH 1/3] fix(iroh): evict cached mapped addrs when a RemoteStateActor shuts down AddrMap (the per-remote endpoint/relay address cache behind mapped_addrs) has no eviction path and is never pruned. On clean RemoteStateActor shutdown, remove_or_restart_actor removed the senders entry but left the remote's cached addresses in endpoint_addrs/relay_addrs forever, so they grew once per remote-ever-seen under churn. Add remove()/retain() to AddrMap (keeping the forward addrs and reverse lookup maps consistent) and evict the departing remote's cached addresses on the clean-shutdown path. Add a unit test for remove/retain forward+reverse consistency. --- iroh/src/socket/mapped_addrs.rs | 59 +++++++++++++++++++++++++++++++++ iroh/src/socket/remote_map.rs | 6 +++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/iroh/src/socket/mapped_addrs.rs b/iroh/src/socket/mapped_addrs.rs index 33d682f748d..790bdedd53b 100644 --- a/iroh/src/socket/mapped_addrs.rs +++ b/iroh/src/socket/mapped_addrs.rs @@ -364,6 +364,31 @@ where let inner = self.inner.lock().expect("poisoned"); inner.lookup.get(addr).cloned() } + + pub(super) fn remove(&self, key: &K) -> Option { + let mut inner = self.inner.lock().expect("poisoned"); + if let Some(v) = inner.addrs.remove(key) { + inner.lookup.remove(&v); + Some(v) + } else { + None + } + } + + pub(super) fn retain(&self, mut f: impl FnMut(&K, &V) -> bool) { + let mut inner = self.inner.lock().expect("poisoned"); + // Disjoint borrows: `addrs.retain` holds `addrs` mutably while the + // closure also needs `lookup` — destructure so they aren't both + // routed through `inner` (avoids E0499). + let AddrMapInner { addrs, lookup } = &mut *inner; + addrs.retain(|k, v| { + let keep = f(k, v); + if !keep { + lookup.remove(v); + } + keep + }); + } } #[derive(Debug)] @@ -381,3 +406,37 @@ impl Default for AddrMapInner { } } } + +#[cfg(test)] +mod tests { + use super::*; + + // The map is generic over the key, so a plain `u32` key exercises the + // forward/reverse bookkeeping; `EndpointIdMappedAddr::generate()` is a + // process-global counter with no other dependencies. + #[test] + fn remove_clears_forward_and_reverse_maps() { + let map: AddrMap = AddrMap::default(); + let addr = map.get(&1); + assert_eq!(map.lookup(&addr), Some(1), "reverse lookup present after insert"); + + assert_eq!(map.remove(&1), Some(addr), "remove returns the mapped addr"); + assert_eq!(map.lookup(&addr), None, "reverse lookup cleared on remove"); + + // Re-inserting generates a fresh addr, proving the old entry was gone. + let addr2 = map.get(&1); + assert_ne!(addr, addr2); + } + + #[test] + fn retain_drops_unmatched_entries_and_their_reverse() { + let map: AddrMap = AddrMap::default(); + let kept = map.get(&1); + let dropped = map.get(&2); + + map.retain(|k, _| *k == 1); + + assert_eq!(map.lookup(&kept), Some(1), "retained entry keeps its reverse lookup"); + assert_eq!(map.lookup(&dropped), None, "dropped entry reverse lookup is cleared"); + } +} diff --git a/iroh/src/socket/remote_map.rs b/iroh/src/socket/remote_map.rs index 37c872cf0d7..db82f3874ab 100644 --- a/iroh/src/socket/remote_map.rs +++ b/iroh/src/socket/remote_map.rs @@ -236,7 +236,11 @@ impl RemoteMap { if leftover_msgs.is_empty() { // the actor shut down cleanly self.senders.remove(&remote_id); - trace!(%remote_id, "cleaned up RemoteStateActor"); + self.mapped_addrs.endpoint_addrs.remove(&remote_id); + self.mapped_addrs + .relay_addrs + .retain(|k, _| k.1 != remote_id); + trace!(%remote_id, "cleaned up RemoteStateActor and mapped_addrs"); true } else { // The remote actor got messages while it was closing, so we're restarting From ef065cd4c1920dd0d4977476b29b4320edb63558 Mon Sep 17 00:00:00 2001 From: Dr Luke Angel Date: Mon, 1 Jun 2026 11:20:51 -0400 Subject: [PATCH 2/3] fix: address PR feedback for iroh --- .containerignore | 2 ++ iroh/src/socket/mapped_addrs.rs | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) create mode 100644 .containerignore diff --git a/.containerignore b/.containerignore new file mode 100644 index 00000000000..3ea08526b04 --- /dev/null +++ b/.containerignore @@ -0,0 +1,2 @@ +target/ +.git/ diff --git a/iroh/src/socket/mapped_addrs.rs b/iroh/src/socket/mapped_addrs.rs index 790bdedd53b..dfdda76ff84 100644 --- a/iroh/src/socket/mapped_addrs.rs +++ b/iroh/src/socket/mapped_addrs.rs @@ -377,9 +377,6 @@ where pub(super) fn retain(&self, mut f: impl FnMut(&K, &V) -> bool) { let mut inner = self.inner.lock().expect("poisoned"); - // Disjoint borrows: `addrs.retain` holds `addrs` mutably while the - // closure also needs `lookup` — destructure so they aren't both - // routed through `inner` (avoids E0499). let AddrMapInner { addrs, lookup } = &mut *inner; addrs.retain(|k, v| { let keep = f(k, v); @@ -411,9 +408,6 @@ impl Default for AddrMapInner { mod tests { use super::*; - // The map is generic over the key, so a plain `u32` key exercises the - // forward/reverse bookkeeping; `EndpointIdMappedAddr::generate()` is a - // process-global counter with no other dependencies. #[test] fn remove_clears_forward_and_reverse_maps() { let map: AddrMap = AddrMap::default(); From 82f27d94494550fdd21a508eec5437da3187a7d4 Mon Sep 17 00:00:00 2001 From: Dr Luke Angel Date: Tue, 2 Jun 2026 06:02:48 -0400 Subject: [PATCH 3/3] style: format mapped_addrs.rs --- iroh/src/socket/mapped_addrs.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/iroh/src/socket/mapped_addrs.rs b/iroh/src/socket/mapped_addrs.rs index dfdda76ff84..595cb311656 100644 --- a/iroh/src/socket/mapped_addrs.rs +++ b/iroh/src/socket/mapped_addrs.rs @@ -412,7 +412,11 @@ mod tests { fn remove_clears_forward_and_reverse_maps() { let map: AddrMap = AddrMap::default(); let addr = map.get(&1); - assert_eq!(map.lookup(&addr), Some(1), "reverse lookup present after insert"); + assert_eq!( + map.lookup(&addr), + Some(1), + "reverse lookup present after insert" + ); assert_eq!(map.remove(&1), Some(addr), "remove returns the mapped addr"); assert_eq!(map.lookup(&addr), None, "reverse lookup cleared on remove"); @@ -430,7 +434,15 @@ mod tests { map.retain(|k, _| *k == 1); - assert_eq!(map.lookup(&kept), Some(1), "retained entry keeps its reverse lookup"); - assert_eq!(map.lookup(&dropped), None, "dropped entry reverse lookup is cleared"); + assert_eq!( + map.lookup(&kept), + Some(1), + "retained entry keeps its reverse lookup" + ); + assert_eq!( + map.lookup(&dropped), + None, + "dropped entry reverse lookup is cleared" + ); } }