diff --git a/include/kpn/network.hpp b/include/kpn/network.hpp index 281b9fa..7af55de 100644 --- a/include/kpn/network.hpp +++ b/include/kpn/network.hpp @@ -8,8 +8,10 @@ #include #endif +#include #include #include +#include #include #include #include @@ -433,9 +435,20 @@ private: void start_watchdog() { watchdog_ = std::jthread([this](std::stop_token tok) { + // Interruptible wait, not sleep_for. request_stop() cannot wake a + // sleeping thread, so stop_watchdog()'s join blocked for up to a + // full interval — three seconds by default, and unbounded for + // anyone who set a long one to keep the periodic report quiet. + // Every teardown paid it. + std::mutex m; + std::condition_variable_any cv; while (!tok.stop_requested()) { - std::this_thread::sleep_for(watchdog_interval_); - if (tok.stop_requested()) break; + { + std::unique_lock lk(m); + if (cv.wait_for(lk, tok, watchdog_interval_, + [&tok] { return tok.stop_requested(); })) + break; + } auto s = collect_snapshots(); check_hung_nodes(); diff --git a/tests/test_network.cpp b/tests/test_network.cpp index 9d75f64..59b6659 100644 --- a/tests/test_network.cpp +++ b/tests/test_network.cpp @@ -1,6 +1,10 @@ #include #include +#include #include +#include +#include +#include #include using namespace kpn; @@ -54,3 +58,36 @@ TEST_CASE("stop disables input channels — producer push is silently dropped", in_ch.push(99); REQUIRE(in_ch.size() == 0); } + +// Regression: stopping a network must not wait for the watchdog's next tick. +// +// The watchdog looped on std::this_thread::sleep_for(watchdog_interval_), and +// request_stop() cannot wake a sleeping thread — so stop_watchdog()'s join +// blocked until the current sleep expired. Every teardown paid up to a full +// interval, three seconds by default, and a caller who set a long one to keep +// the periodic report quiet got a stop() that looked like a hang. That is how +// this was found: the error-handler case above set an hour. +TEST_CASE("stopping a network does not wait for the watchdog interval", "[network]") { + auto node = kpn::make_node(kpn::in<"v">{}, kpn::out<"w">{}, 4); + kpn::Channel out(4); + node.set_output_channel<0>(&out); + + kpn::Network net; + net.add("inc", node).build(); + net.set_watchdog_interval(std::chrono::hours(1)); + net.start(); + + // Let the watchdog actually reach its wait. Without this the test races it: + // stop_watchdog() runs before the thread has entered the loop, the token is + // already set when it does, and it exits without ever waiting — which passes + // against the bug as well as the fix. + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + + const auto t0 = std::chrono::steady_clock::now(); + net.stop(); + const auto ms = std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + + INFO("stop took " << ms << " ms"); + CHECK(ms < 2000); +}