Merge maint into master, carrying the bloom re-announce, BLE warning and child-exit reporting fixes

This commit is contained in:
Johnathan Corgan
2026-09-23 19:37:13 +00:00
7 changed files with 736 additions and 68 deletions
+27
View File
@@ -383,6 +383,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
#### Node lifecycle
- A DNS responder or TUN thread that dies now degrades the node's published
health, and a dead responder's address is retracted. The responder's exit
report followed a loop that never returns, so it could not run, and a panic
in the responder or in either TUN thread unwound past its report. The node
kept reporting healthy with the child gone and kept publishing the DNS
address with nothing answering on it. Each child now runs inside a wrapper
that catches a panic, logs it, and reports the exit either way. A deliberate
stop still reports nothing.
- Losing an interface no longer leaves its peers in the routing table. The
peers stayed in the registry, the routes through them stayed selectable, and
the node kept advertising reachability it no longer had — so transit traffic
@@ -444,6 +453,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
#### Data plane and transports
- A configured `ble:` transport that this build cannot construct is now
reported. The only warning for it was compiled into test builds alone, where
logging is compiled out, so macOS, Windows, FreeBSD, OpenWrt and other musl
builds, and Android without a BLE radio armed before start, dropped the block
silently while reporting healthy. The daemon now warns once per configured
instance at startup, naming the reason.
- A peer that stops reading can no longer stall the node. TCP, Tor, Nym and
BLE wrote to their links directly from the caller's task, and a write
blocks once the peer's receive window and this node's send buffer are both
@@ -542,6 +558,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
interval instead. That retry interval gates only a peer whose last attempt
failed, so it cannot clamp a `heartbeat_interval_secs` configured below it.
#### Routing and discovery
- A node now re-announces its bloom filter when a peer starts or stops using
it as parent. A node's outgoing filter merges only its tree peers' filters,
and nothing re-marked the other peers when a peer's tree announce changed
whether it named us as parent, so our parent kept the old filter.
Destinations under a new child stayed missing from discovery, and a departed
child's stayed advertised, until some unrelated change. The re-announce fires
only when that relation flips and only to peers whose filter actually
changed, so ordinary tree churn does not multiply announce traffic.
#### Session setup
- A session whose last handshake message is lost no longer stays one-sided.
+101 -52
View File
@@ -25,7 +25,7 @@ use std::collections::{HashMap, HashSet};
use std::net::SocketAddr;
use std::thread;
use std::time::Duration;
use tracing::{debug, info, warn};
use tracing::{debug, error, info, warn};
const OPEN_DISCOVERY_RETRY_LIFETIME_MULTIPLIER: u64 = 2;
const MAX_PARALLEL_PATH_CANDIDATES_PER_PEER: usize = 4;
@@ -38,6 +38,48 @@ fn socket_addr_families_compatible(local: SocketAddr, remote: SocketAddr) -> boo
)
}
/// Run a supervised child's async body, then report the child's exit on `tx`,
/// whether the body returned or panicked.
///
/// A panic would otherwise unwind past the report and leave the node healthy
/// with the child gone. Aborting the task still reports nothing: the abort
/// drops this whole future, so a deliberate stop does not read as a death.
pub(in crate::node) async fn report_exit(
child: Child,
body: impl std::future::Future<Output = ()>,
tx: Option<tokio::sync::mpsc::Sender<Child>>,
) {
use futures::FutureExt;
if std::panic::AssertUnwindSafe(body)
.catch_unwind()
.await
.is_err()
{
// The panic hook has already printed the payload.
error!(child = ?child, "Supervised child panicked; reporting its exit");
}
if let Some(tx) = tx {
let _ = tx.send(child).await;
}
}
/// Run a supervised child's thread body, then report the child's exit on `tx`,
/// whether the body returned or panicked.
pub(in crate::node) fn report_thread(
child: Child,
body: impl FnOnce(),
tx: Option<&tokio::sync::mpsc::Sender<Child>>,
) {
if std::panic::catch_unwind(std::panic::AssertUnwindSafe(body)).is_err() {
// The panic hook has already printed the payload.
error!(child = ?child, "Supervised child panicked; reporting its exit");
}
if let Some(tx) = tx {
let _ = tx.blocking_send(child);
}
}
impl Node {
/// Replace the runtime peer list.
///
@@ -1715,16 +1757,18 @@ impl Node {
let (writer, tun_tx) = device
.create_writer(max_mss.clone(), self.path_mtu_lookup.clone())?;
// Spawn writer thread. On exit it self-reports
// `Child::Tun` (sync context → `blocking_send`); TUN
// is one compound child, so both threads reporting is
// fine (the FSM de-dups via `up.remove`).
// Spawn writer thread. On exit, including a panic,
// it self-reports `Child::Tun` (sync context →
// `blocking_send`); TUN is one compound child, so
// both threads reporting is fine (the FSM de-dups
// via `up.remove`).
let writer_child_tx = self.child_exit_tx.clone();
let writer_handle = thread::spawn(move || {
writer.run();
if let Some(tx) = &writer_child_tx {
let _ = tx.blocking_send(Child::Tun);
}
report_thread(
Child::Tun,
move || writer.run(),
writer_child_tx.as_ref(),
);
});
// Clone tun_tx for the reader
@@ -1736,41 +1780,48 @@ impl Node {
tokio::sync::mpsc::channel(tun_channel_size);
// Spawn reader thread. Like the writer, it
// self-reports `Child::Tun` on exit (sync context →
// `blocking_send`). Exactly one cfg variant compiles,
// so the single clone is moved into that closure.
// self-reports `Child::Tun` on exit or panic (sync
// context → `blocking_send`). Exactly one cfg
// variant compiles, so the single clone is moved
// into that closure.
let path_mtu_lookup = self.path_mtu_lookup.clone();
let reader_child_tx = self.child_exit_tx.clone();
#[cfg(any(target_os = "macos", target_os = "freebsd"))]
let reader_handle = thread::spawn(move || {
run_tun_reader(
device,
mtu,
our_addr,
reader_tun_tx,
outbound_tx,
max_mss,
path_mtu_lookup,
shutdown_read_fd,
report_thread(
Child::Tun,
move || {
run_tun_reader(
device,
mtu,
our_addr,
reader_tun_tx,
outbound_tx,
max_mss,
path_mtu_lookup,
shutdown_read_fd,
)
},
reader_child_tx.as_ref(),
);
if let Some(tx) = &reader_child_tx {
let _ = tx.blocking_send(Child::Tun);
}
});
#[cfg(not(any(target_os = "macos", target_os = "freebsd")))]
let reader_handle = thread::spawn(move || {
run_tun_reader(
device,
mtu,
our_addr,
reader_tun_tx,
outbound_tx,
max_mss,
path_mtu_lookup,
report_thread(
Child::Tun,
move || {
run_tun_reader(
device,
mtu,
our_addr,
reader_tun_tx,
outbound_tx,
max_mss,
path_mtu_lookup,
)
},
reader_child_tx.as_ref(),
);
if let Some(tx) = &reader_child_tx {
let _ = tx.blocking_send(Child::Tun);
}
});
self.tun_state = TunState::Active;
@@ -1860,23 +1911,24 @@ impl Node {
);
// Self-report on exit so the supervisor FSM
// routes health when the DNS task dies at
// runtime. On a deliberate stop the task is
// `.abort()`ed before this send; even if it
// fired, the FSM ignores it outside `Running`.
// runtime. The responder never returns, so
// in practice that is a panic. On a
// deliberate stop the task is `.abort()`ed,
// which drops the report with it; even if
// one fired, the FSM ignores it outside
// `Running`.
let dns_child_tx = self.child_exit_tx.clone();
let handle = tokio::spawn(async move {
let handle = tokio::spawn(report_exit(
Child::Dns,
crate::upper::dns::run_dns_responder(
socket,
identity_tx,
dns_ttl,
reloader,
mesh_ifindex,
)
.await;
if let Some(tx) = dns_child_tx {
let _ = tx.send(Child::Dns).await;
}
});
),
dns_child_tx,
));
self.supervisor.dns_identity_rx = Some(identity_rx);
self.supervisor.dns_task = Some(handle);
self.supervisor.dns_local_addr = Some(local_addr);
@@ -2310,13 +2362,10 @@ impl Node {
/// reads it to rebuild the teardown set, and aborting an already-finished
/// handle there is harmless.
///
/// **Dormant for `Dns` as written.** `run_dns_responder` is an unconditional
/// loop whose every failure arm continues, so it never returns and the
/// `Child::Dns` send that follows it is unreachable — nothing produces the
/// event this consumes. The consumer side is correct and lands here so the
/// producer fix does not have to rediscover it. A responder that *panics* is
/// not covered either way, since the unwind goes past the send rather than
/// through it; that is true of every child producer, not just this one.
/// For `Dns` the event comes only from a panic. `run_dns_responder` is an
/// unconditional loop whose every failure arm continues, so it has no
/// ordinary exit; [`report_exit`] catches a panic in it and reports
/// `Child::Dns`, which is what reaches this.
pub(in crate::node) fn retract_child_publications(&mut self, child: Child) {
if matches!(child, Child::Dns) {
self.supervisor.dns_local_addr.take();
+47 -8
View File
@@ -1254,7 +1254,7 @@ impl Node {
}
// Create BLE transport instances
#[cfg(bluer_available)]
#[cfg(all(bluer_available, not(test)))]
{
let ble_instances: Vec<_> = self
.config()
@@ -1264,7 +1264,6 @@ impl Node {
.map(|(name, config)| (name.map(|s| s.to_string()), config.clone()))
.collect();
#[cfg(all(bluer_available, not(test)))]
for (name, ble_config) in ble_instances {
let transport_id = self.allocate_transport_id();
let adapter = ble_config.adapter().to_string();
@@ -1286,12 +1285,6 @@ impl Node {
}
}
}
#[cfg(any(not(bluer_available), test))]
if !ble_instances.is_empty() {
#[cfg(not(test))]
tracing::warn!("BLE transport configured but this build lacks BlueZ support");
}
}
// Create BLE transport instances over an embedder-supplied radio.
@@ -1320,10 +1313,56 @@ impl Node {
transports.push(TransportHandle::Ble(ble));
}
}
// `BleConfig` always parses, so on a build that cannot construct a
// BLE transport a configured `ble:` block would otherwise be dropped
// silently and the node would report healthy without it.
if let Some(reason) = self.ble_blocker() {
for (name, _) in self.config().transports.ble.iter() {
tracing::warn!(
instance = name.unwrap_or("default"),
reason,
"BLE transport unavailable; ignoring configured instance"
);
}
}
transports
}
/// Why this build cannot construct a configured BLE instance, or `None`
/// when it can.
///
/// The three arms are disjoint and together cover every build, so a
/// target matching none or two of them fails to compile rather than
/// guessing.
#[cfg(all(bluer_available, not(test)))]
fn ble_blocker(&self) -> Option<&'static str> {
None
}
/// Why this build cannot construct a configured BLE instance, or `None`
/// when it can. The embedder-supplied backend needs its radio slot armed
/// before `start()`.
#[cfg(all(target_os = "android", not(bluer_available), not(test)))]
fn ble_blocker(&self) -> Option<&'static str> {
if self.ble_radio.is_some() {
None
} else {
Some("no BLE radio was armed before start")
}
}
/// Why this build cannot construct a configured BLE instance: it has no
/// backend at all. A test build lands here too, since its BLE transport
/// is the in-memory double and is never built from config.
#[cfg(not(any(
all(bluer_available, not(test)),
all(target_os = "android", not(bluer_available), not(test))
)))]
fn ble_blocker(&self) -> Option<&'static str> {
Some("this build has no BLE backend")
}
/// Find an operational transport that matches the given transport type name.
fn find_transport_for_type(&self, transport_type: &str) -> Option<TransportId> {
self.transports
+270
View File
@@ -718,3 +718,273 @@ async fn test_bloom_filter_convergence_100_nodes() {
print_filter_cardinality(&nodes);
cleanup_nodes(&mut nodes).await;
}
// ===== Outgoing filter re-announce on a tree-peer flip =====
//
// These tests read only what M actually sent (`last_sent_filter`, written
// after a successful send) or what M has marked (`needs_update`). Recomputing
// an outgoing filter would read live peer state and bypass the marking under
// test, so none of them does.
use crate::node::tree::sign_declaration;
use crate::proto::bloom::{BloomFilter, FilterAnnounce};
use crate::proto::stp::{ParentDeclaration, TreeAnnounce, TreeCoordinate};
/// Three loopback nodes, P -- M -- C, with M's tree view forced so that P is
/// M's parent and C either names M as parent (a child) or names an unrelated
/// node (not a tree peer).
struct FlipFixture {
nodes: Vec<TestNode>,
m: NodeAddr,
p: NodeAddr,
c: NodeAddr,
root: NodeAddr,
fake: NodeAddr,
}
/// Index of M in `FlipFixture::nodes`.
const M: usize = 1;
/// Index of C in `FlipFixture::nodes`.
const C: usize = 2;
/// Synthetic address carried in C's filter and in no real one.
fn marker() -> NodeAddr {
make_node_addr(0xab)
}
/// Build the fixture and flush whatever convergence left pending at M.
///
/// With `child_start`, C's stored declaration names M as parent; otherwise it
/// names `fake`. Every stored declaration uses sequence 1, so the sequence-5
/// announces from `deliver_tree_announce` are fresher.
async fn flip_fixture(child_start: bool) -> FlipFixture {
let nodes = run_tree_test(3, &[(0, 1), (1, 2)], false).await;
let p = *nodes[0].node.node_addr();
let m = *nodes[M].node.node_addr();
let c = *nodes[C].node.node_addr();
// All zero bytes: smaller than any real address, so it stays root.
let root = make_node_addr(0);
let fake = make_node_addr(1);
let mut fx = FlipFixture {
nodes,
m,
p,
c,
root,
fake,
};
{
let ts = fx.nodes[M].node.tree_state_mut();
ts.remove_peer(&p);
ts.update_peer(
ParentDeclaration::new(p, root, 1, 1000),
TreeCoordinate::from_addrs(vec![p, root]).unwrap(),
);
ts.set_parent(p, 1, 1000, 1000);
ts.recompute_coords();
ts.remove_peer(&c);
let (parent, coords) = if child_start {
(m, vec![c, m, p, root])
} else {
(fake, vec![c, fake, root])
};
ts.update_peer(
ParentDeclaration::new(c, parent, 1, 1000),
TreeCoordinate::from_addrs(coords).unwrap(),
);
}
{
let identity = fx.nodes[M].node.identity().clone();
let decl_mut = fx.nodes[M].node.tree_state_mut().my_declaration_mut();
sign_declaration(decl_mut, &identity).unwrap();
}
// Debounce is a brake, not the mechanism under test: send on every drain.
fx.nodes[M].node.bloom_state.set_update_debounce_ms(0);
fx.nodes[M].node.send_pending_filter_announces().await;
let node = &fx.nodes[M].node;
assert_eq!(
node.is_tree_peer(&c),
child_start,
"setup: C's starting tree-peer state"
);
assert_eq!(
node.tree_state().my_declaration().parent_id(),
&p,
"setup: M's parent must be P"
);
fx
}
/// Deliver a FilterAnnounce from C to M holding exactly `addrs`, and check M
/// stored it, so a rejected announce cannot decide a later assertion.
async fn deliver_filter(fx: &mut FlipFixture, addrs: &[NodeAddr]) {
let c = fx.c;
let mut filter = BloomFilter::new();
for addr in addrs {
filter.insert(addr);
}
let seq = fx.nodes[M].node.get_peer(&c).unwrap().filter_sequence() + 1;
let mut payload = FilterAnnounce::new(filter.clone(), seq).encode().unwrap();
payload.remove(0); // strip msg_type byte
fx.nodes[M].node.handle_filter_announce(&c, &payload).await;
let stored = fx.nodes[M].node.get_peer(&c).unwrap().inbound_filter();
assert_eq!(
stored,
Some(&filter),
"setup: M must store C's delivered filter"
);
}
/// Deliver a signed sequence-5 TreeAnnounce from C to M through the real
/// handler, declaring `parent` with ancestry `coords`. Checks the announce was
/// accepted and that M did not switch parent, since a switch marks every peer
/// and would pass the tests for a reason unrelated to the flip.
async fn deliver_tree_announce(fx: &mut FlipFixture, parent: NodeAddr, coords: Vec<NodeAddr>) {
let c = fx.c;
let mut decl = ParentDeclaration::new(c, parent, 5, 2000);
sign_declaration(&mut decl, fx.nodes[C].node.identity()).unwrap();
let announce = TreeAnnounce::new(decl, TreeCoordinate::from_addrs(coords).unwrap());
let encoded = announce.encode().unwrap();
let node = &mut fx.nodes[M].node;
let accepted_before = node.metrics().tree.accepted.get();
let switches_before = node.metrics().tree.parent_switches.get();
node.handle_tree_announce(&c, &encoded[1..]).await;
assert_eq!(
node.metrics().tree.accepted.get(),
accepted_before + 1,
"setup: C's tree announce must be accepted"
);
assert_eq!(
node.tree_state().my_declaration().parent_id(),
&fx.p,
"setup: M's parent must still be P"
);
assert_eq!(
node.metrics().tree.parent_switches.get(),
switches_before,
"setup: M must not switch parent"
);
}
/// The filter M last sent to P, which must exist.
fn sent_to_parent(fx: &FlipFixture) -> &BloomFilter {
fx.nodes[M]
.node
.bloom_state
.last_sent_filter(&fx.p)
.expect("M has sent a filter to P")
}
/// When C starts naming M as parent, M must re-announce to P a filter that
/// now includes C's contribution.
#[tokio::test]
async fn test_bloom_outgoing_filter_to_parent_reannounced_when_peer_becomes_child() {
let mut fx = flip_fixture(false).await;
let (m, c, p, root) = (fx.m, fx.c, fx.p, fx.root);
deliver_filter(&mut fx, &[c, marker()]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
assert!(
!sent_to_parent(&fx).contains(&marker()),
"control: a non-tree peer's filter must not reach P"
);
deliver_tree_announce(&mut fx, m, vec![c, m, p, root]).await;
assert!(fx.nodes[M].node.is_tree_peer(&c));
fx.nodes[M].node.send_pending_filter_announces().await;
assert!(
sent_to_parent(&fx).contains(&marker()),
"P must be sent C's filter once C becomes M's child"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// When C stops naming M as parent, M must re-announce to P a filter that no
/// longer includes C's contribution.
#[tokio::test]
async fn test_bloom_outgoing_filter_to_parent_reannounced_when_child_leaves() {
let mut fx = flip_fixture(true).await;
let (c, root, fake) = (fx.c, fx.root, fx.fake);
deliver_filter(&mut fx, &[c, marker()]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
assert!(
sent_to_parent(&fx).contains(&marker()),
"control: a child's filter must reach P"
);
deliver_tree_announce(&mut fx, fake, vec![c, fake, root]).await;
assert!(!fx.nodes[M].node.is_tree_peer(&c));
fx.nodes[M].node.send_pending_filter_announces().await;
assert!(
!sent_to_parent(&fx).contains(&marker()),
"P must stop being sent C's filter once C leaves"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// A child's filter that arrives after its tree announce reaches the parent
/// through the ordinary filter-announce marking.
#[tokio::test]
async fn test_bloom_child_filter_after_tree_announce_reaches_parent() {
let mut fx = flip_fixture(false).await;
let (m, c, p, root) = (fx.m, fx.c, fx.p, fx.root);
deliver_tree_announce(&mut fx, m, vec![c, m, p, root]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
deliver_filter(&mut fx, &[c, marker()]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
assert!(
sent_to_parent(&fx).contains(&marker()),
"P must be sent a child's filter delivered after its tree announce"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// A tree-peer flip that changes no outgoing filter marks no peer: C's filter
/// holds only M's own address, which M's base filter already carries.
#[tokio::test]
async fn test_bloom_tree_peer_flip_without_filter_change_marks_no_peer() {
let mut fx = flip_fixture(false).await;
let (m, c, p, root) = (fx.m, fx.c, fx.p, fx.root);
deliver_filter(&mut fx, &[m]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
let bloom = &fx.nodes[M].node.bloom_state;
assert!(!bloom.needs_update(&p) && !bloom.needs_update(&c));
deliver_tree_announce(&mut fx, m, vec![c, m, p, root]).await;
assert!(fx.nodes[M].node.is_tree_peer(&c));
let bloom = &fx.nodes[M].node.bloom_state;
assert!(!bloom.needs_update(&p), "P must not be marked");
assert!(!bloom.needs_update(&c), "C must not be marked");
cleanup_nodes(&mut fx.nodes).await;
}
/// A fresher tree announce that leaves C a child of M marks no peer.
#[tokio::test]
async fn test_bloom_tree_announce_without_tree_peer_flip_marks_no_peer() {
let mut fx = flip_fixture(true).await;
let (m, c, p, root) = (fx.m, fx.c, fx.p, fx.root);
deliver_filter(&mut fx, &[c, marker()]).await;
fx.nodes[M].node.send_pending_filter_announces().await;
let bloom = &fx.nodes[M].node.bloom_state;
assert!(!bloom.needs_update(&p) && !bloom.needs_update(&c));
deliver_tree_announce(&mut fx, m, vec![c, m, p, root]).await;
assert!(fx.nodes[M].node.is_tree_peer(&c));
let bloom = &fx.nodes[M].node.bloom_state;
assert!(!bloom.needs_update(&p), "P must not be marked");
assert!(!bloom.needs_update(&c), "C must not be marked");
cleanup_nodes(&mut fx.nodes).await;
}
+259 -5
View File
@@ -3786,6 +3786,74 @@ fn app_owned_ble_radio_seam_is_absent_until_armed() {
assert!(node.ble_radio.is_none());
}
/// A configured `ble:` block that this build cannot turn into a transport is
/// named in a warning, once per instance, rather than dropped silently.
///
/// `BleConfig` parses on every platform, so a node on a build with no BLE
/// backend (macOS, Windows, FreeBSD, musl, and a test build) would otherwise
/// start and report healthy with the configured radio simply absent. Not
/// `cfg`-gated: the warning is for exactly the builds that lack BLE.
#[tokio::test]
async fn configured_ble_instances_each_draw_a_warning_when_no_backend_can_build_them() {
let mut config = crate::Config::new();
config.node.control.enabled = false;
config.transports.ble = crate::config::TransportInstances::Named(
[
("alpha".to_string(), crate::config::BleConfig::default()),
("beta".to_string(), crate::config::BleConfig::default()),
]
.into_iter()
.collect(),
);
let mut node = make_node_with(config);
let (tx, _rx) = packet_channel(8);
let (logs, guard) = crate::testutil::capture_logs_scoped();
let transports = node.create_transports(&tx).await;
drop(guard);
assert!(
transports.is_empty(),
"no transport can be built from a BLE block on this build",
);
let warnings = logs.warnings();
for instance in ["alpha", "beta"] {
let field = format!("instance=\"{instance}\"");
let hits = warnings
.iter()
.filter(|line| line.contains("ignoring configured instance") && line.contains(&field))
.count();
assert_eq!(
hits, 1,
"expected one warning naming BLE instance {instance}, got {warnings:?}",
);
}
}
/// The healthy side of the check above: with no `ble:` block there is nothing
/// to warn about, so the new warning must not fire on an ordinary config.
#[tokio::test]
async fn a_node_without_ble_config_draws_no_ble_warning() {
let mut config = crate::Config::new();
config.node.control.enabled = false;
let mut node = make_node_with(config);
let (tx, _rx) = packet_channel(8);
let (logs, guard) = crate::testutil::capture_logs_scoped();
let transports = node.create_transports(&tx).await;
drop(guard);
assert!(
transports.is_empty(),
"the default config builds no transport"
);
let warnings = logs.warnings();
assert!(
!warnings.iter().any(|line| line.contains("BLE transport")),
"no BLE block is configured, got {warnings:?}",
);
}
#[cfg(all(ble_available, any(target_os = "android", test)))]
mod test_radio {
use crate::transport::ble::addr::BleAddr;
@@ -3923,12 +3991,12 @@ async fn dns_responder_serves_a_proxying_embedder() {
///
/// Scoped to the helper deliberately, and named for that rather than for the
/// scenario: no responder dies here, and deleting the `run_rx_loop` call site
/// leaves this green. Driving a real exit through the loop needs the node moved
/// into a task, which puts `dns_local_addr()` out of reach — and the producer
/// side cannot deliver `Child::Dns` today regardless, since `run_dns_responder`
/// never returns.
/// leaves this green. The rx-loop wiring is pinned separately, by
/// `a_dns_exit_on_the_child_channel_degrades_the_node_and_retracts_its_address`.
/// The producer side reports `Child::Dns` only when the responder panics, since
/// `run_dns_responder` has no ordinary exit.
///
/// What it does pin is the behavior the eventual wiring depends on: the FSM's
/// What it does pin is the behavior the wiring depends on: the FSM's
/// `ChildExited` handling only republishes node health, so without this
/// retraction `dns_local_addr()` would keep naming a socket nobody is listening
/// on, and a proxying embedder would see `.fips` queries silently time out
@@ -3959,6 +4027,192 @@ async fn retract_child_publications_clears_the_dns_address() {
node.stop().await.unwrap();
}
/// A supervised task that panics is still reported to the supervisor.
///
/// Without the report the panic ends the task silently: the `JoinError` sits
/// on a handle nobody reads until `stop()`, and the node stays `Running` with
/// the child gone.
#[tokio::test]
async fn report_exit_reports_a_body_that_panics() {
use crate::node::lifecycle::report_exit;
use crate::node::lifecycle::supervisor::Child;
let (tx, mut rx) = tokio::sync::mpsc::channel(1);
let handle = tokio::spawn(report_exit(
Child::Dns,
async { panic!("the supervised body died") },
Some(tx),
));
let joined = handle.await;
assert_eq!(
rx.recv().await,
Some(Child::Dns),
"a panicking child must still report its exit",
);
assert!(
joined.is_ok(),
"the panic must be contained by the reporter"
);
}
/// A supervised task whose body returns is reported, so a future ordinary
/// exit from the DNS responder needs no further wiring.
#[tokio::test]
async fn report_exit_reports_a_body_that_returns() {
use crate::node::lifecycle::report_exit;
use crate::node::lifecycle::supervisor::Child;
let (tx, mut rx) = tokio::sync::mpsc::channel(1);
tokio::spawn(report_exit(Child::Dns, async {}, Some(tx)))
.await
.unwrap();
assert_eq!(rx.recv().await, Some(Child::Dns));
}
/// A deliberate teardown aborts a running child, and that must not read as a
/// death: the abort drops the whole reporting future before any send.
#[tokio::test]
async fn report_exit_stays_silent_when_the_task_is_aborted() {
use crate::node::lifecycle::report_exit;
use crate::node::lifecycle::supervisor::Child;
let (tx, mut rx) = tokio::sync::mpsc::channel(1);
let (started_tx, started_rx) = tokio::sync::oneshot::channel();
let handle = tokio::spawn(report_exit(
Child::Dns,
async move {
let _ = started_tx.send(());
std::future::pending::<()>().await;
},
Some(tx),
));
// The body is running, as a live responder is when `stop()` aborts it.
started_rx.await.unwrap();
handle.abort();
assert!(handle.await.unwrap_err().is_cancelled());
assert_eq!(rx.recv().await, None, "an aborted child must not report");
}
/// A supervised thread that panics is still reported to the supervisor.
#[test]
fn report_thread_reports_a_body_that_panics() {
use crate::node::lifecycle::report_thread;
use crate::node::lifecycle::supervisor::Child;
let (tx, mut rx) = tokio::sync::mpsc::channel(1);
let joined = std::thread::spawn(move || {
report_thread(
Child::Tun,
|| panic!("the supervised thread died"),
Some(&tx),
)
})
.join();
assert_eq!(
rx.try_recv(),
Ok(Child::Tun),
"a panicking thread must still report its exit",
);
assert!(
joined.is_ok(),
"the panic must be contained by the reporter"
);
}
/// A supervised thread whose body returns is reported.
#[test]
fn report_thread_reports_a_body_that_returns() {
use crate::node::lifecycle::report_thread;
use crate::node::lifecycle::supervisor::Child;
let (tx, mut rx) = tokio::sync::mpsc::channel(1);
std::thread::spawn(move || report_thread(Child::Tun, || {}, Some(&tx)))
.join()
.unwrap();
assert_eq!(rx.try_recv(), Ok(Child::Tun));
}
/// A node with a UDP transport and a DNS responder on an ephemeral port, and
/// no control socket for the rx loop to bind.
fn dns_config() -> crate::Config {
let mut config = crate::Config::new();
config.node.control.enabled = false;
config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig {
bind_addr: Some("127.0.0.1:0".to_string()),
..Default::default()
});
config.dns.enabled = true;
config.dns.bind_addr = Some("::1".to_string());
config.dns.port = Some(0);
config
}
/// A DNS exit reaching the child channel degrades the node and retracts the
/// responder's published address, through the real rx loop.
///
/// The exit is queued before the loop starts and the loop is driven in place
/// under a bound, so the node's state is readable afterwards. The shutdown
/// future never fires, which keeps the loop out of the drain path.
#[tokio::test]
async fn a_dns_exit_on_the_child_channel_degrades_the_node_and_retracts_its_address() {
let mut node = make_node_with(dns_config());
node.start().await.unwrap();
assert_eq!(node.state(), NodeState::Running);
assert!(node.dns_local_addr().is_some(), "responder came up");
node.child_exit_tx
.clone()
.unwrap()
.send(crate::node::lifecycle::supervisor::Child::Dns)
.await
.unwrap();
let drive = tokio::time::timeout(
Duration::from_millis(500),
node.run_rx_loop_with_shutdown(std::future::pending()),
)
.await;
assert!(
drive.is_err(),
"the rx loop must still be running: {drive:?}"
);
assert_eq!(node.state(), NodeState::Degraded);
assert!(
node.dns_local_addr().is_none(),
"a dead responder must not keep publishing an address to dial",
);
node.stop().await.unwrap();
}
/// The healthy side of the wiring test: the same drive with no exit queued
/// leaves the node `Running` and its responder's address published.
#[tokio::test]
async fn an_rx_loop_with_no_child_exit_leaves_the_node_running_and_its_dns_address_published() {
let mut node = make_node_with(dns_config());
node.start().await.unwrap();
let drive = tokio::time::timeout(
Duration::from_millis(500),
node.run_rx_loop_with_shutdown(std::future::pending()),
)
.await;
assert!(
drive.is_err(),
"the rx loop must still be running: {drive:?}"
);
assert_eq!(node.state(), NodeState::Running);
assert!(node.dns_local_addr().is_some());
node.stop().await.unwrap();
}
/// `dns.enabled` with a bind that fails must report `None`, not an address.
///
/// This is the third state an embedder has to tell apart, and the one that
+20 -2
View File
@@ -261,6 +261,11 @@ impl Node {
);
}
// Sample before the TreeState write below: is_tree_peer reads the
// declaration that write stores, so sampling after it would always
// see the new value and never detect a flip.
let was_tree = self.is_tree_peer(from);
// Update in TreeState
let updated = self
.tree_state
@@ -274,6 +279,17 @@ impl Node {
self.metrics().tree.accepted.inc();
// A peer that starts or stops naming us as parent joins or leaves the
// set of filters merged into our outgoing filters, so every other
// peer's outgoing filter may have changed. Mark only the peers whose
// filter actually differs from what was last sent. This runs before
// the parent re-evaluation below so no early return there can skip it.
if self.is_tree_peer(from) != was_tree {
let peer_addrs: Vec<NodeAddr> = self.peers.keys().copied().collect();
let peer_filters = self.peer_inbound_filters();
self.bloom_state.mark_changed(&peer_addrs, &peer_filters);
}
debug!(
from = %self.peer_display_name(from),
seq = announce.declaration.sequence(),
@@ -318,9 +334,11 @@ impl Node {
}
// Bloom filter exchange initiation is handled at handshake completion
// ([handshake.rs] mark_update_needed on the new peer) and on actual
// ([handshake.rs] mark_update_needed on the new peer), on actual
// content changes via [bloom.rs::handle_filter_announce]'s
// `mark_changed_peers`. Marking the peer on every received TreeAnnounce
// `mark_changed_peers`, and above when this peer starts or stops
// naming us as parent (marked by content change only, and only on
// the flip). Marking the peer on every received TreeAnnounce
// is redundant — and under high TreeAnnounce churn (rapid mid-chain
// swap propagation) it amplifies bloom traffic proportionally with
// the tree announce rate, even when the local outgoing filter
+12 -1
View File
@@ -177,8 +177,19 @@ impl BloomState {
.filter(|addr| *addr != exclude_from)
.copied()
.collect();
self.mark_changed(&targets, peer_filters);
}
for (peer_addr, new_filter) in self.compute_outgoing_filters(&targets, peer_filters) {
/// Mark every target whose outgoing filter differs from what was last sent.
///
/// A target never sent to counts as changed. Unlike
/// [`mark_changed_peers`](Self::mark_changed_peers), no peer is excluded.
pub fn mark_changed(
&mut self,
targets: &[NodeAddr],
peer_filters: &BTreeMap<NodeAddr, BloomFilter>,
) {
for (peer_addr, new_filter) in self.compute_outgoing_filters(targets, peer_filters) {
let changed = match self.last_sent_filters.get(&peer_addr) {
Some(last) => *last != new_filter,
None => true, // never sent → must send