From 2caa21d65fc5c1bd60eeec9bda5c317e2e418cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cyrille=20B=C3=A9raud?= Date: Sun, 2 Aug 2026 19:28:10 -0400 Subject: [PATCH] dht: let a node that lost the network try to rejoin it Dht::onDisconnected() only starts a bootstrap when bootstrapJob is unset, reading a set handle as "a bootstrap is already scheduled". That reading is wrong: Scheduler::run() erases the job from its timers and calls it, but never clears the shared handle the caller holds, and Scheduler::cancel() leaves the handle alone too when the job is no longer in the timers. The handle therefore stays set for as long as the Dht lives, once a bootstrap job has run. That is harmless while bootstrap() reschedules itself, which it does as long as the node is disconnected. It is not harmless on the early return: a job that fires while a ping is still in flight, or while a dubious contact keeps the status at Connecting, returns without rescheduling. From then on the chain is dead and onDisconnected() will not restart it, so a node that goes on to lose every contact stays disconnected for good. Only an explicit addBootstrap() or a connectivityChanged() can revive it, and a node whose network simply went away gets neither. Clearing the handle on the early return restores the meaning onDisconnected() relies on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/dht.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dht.cpp b/src/dht.cpp index 273478bc..914407dd 100644 --- a/src/dht.cpp +++ b/src/dht.cpp @@ -2270,8 +2270,16 @@ Dht::onDisconnected() void Dht::bootstrap() { - if (dht4.status != NodeStatus::Disconnected || dht6.status != NodeStatus::Disconnected) + if (dht4.status != NodeStatus::Disconnected || dht6.status != NodeStatus::Disconnected) { + // The job that led here has already run, and the scheduler does not + // clear the handle to a job it has executed. onDisconnected() reads a + // set handle as "a bootstrap is already scheduled" and does nothing, + // so leaving it set here means the node never tries to rejoin the + // network once it has lost it. Clearing it is what says that no + // bootstrap is pending any more. + bootstrapJob.reset(); return; + } if (logger_) logger_->debug("[{}] Bootstraping", myid.to_view()); bootstrap_pending = false;