From 79afdeff8dccb033ffd6d4d763166a3ea6ce9283 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 14:21:08 +0000 Subject: [PATCH] Stop the test mocks deleting cryptLock The mock routers freed the global crypt lock in their destructors, each with a comment pointing at the reason: Router's constructor asserted the global was null before allocating it, so a suite that built more than one router had to hand it back. That assert is gone - the constructor now reuses an existing lock, the same idiom as routingAuthCacheLock on the next line - and nothing in src/ has ever freed it, so these deletes now free a lock the mock does not own and leave the global null behind them. test_admin_radio is where that bites: ~HamModeMockRouter runs from tearDown, and the next test reaches AdminModule's receive path, which takes LockGuard(cryptLock) - a null dereference now that the Portduino Lock is a real mutex. The other six were the same hazard waiting for a suite to add a test after them. Co-Authored-By: Jonathan Bennett Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VeY5PczVYsxWNZhE2Cx4Sv --- test/test_admin_radio/test_main.cpp | 5 ----- test/test_mesh_beacon/test_main.cpp | 6 ------ test/test_mesh_module/test_main.cpp | 6 ------ test/test_mqtt/MQTT.cpp | 6 ------ test/test_nexthop_routing/test_main.cpp | 7 ------- test/test_nodeinfo_send_window/test_main.cpp | 8 -------- test/test_traffic_management/test_main.cpp | 8 -------- 7 files changed, 46 deletions(-) diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index a0cfb416f2..5333f6cb34 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -1220,11 +1220,6 @@ static void test_handleSetHamMode_blankShortNameKeepsTheExistingOne() class HamModeMockRouter : public Router { public: - ~HamModeMockRouter() - { - delete cryptLock; // the Router ctor asserts this is clear, so a later suite can construct one - cryptLock = nullptr; - } ErrorCode send(meshtastic_MeshPacket *p) override { packetPool.release(p); diff --git a/test/test_mesh_beacon/test_main.cpp b/test/test_mesh_beacon/test_main.cpp index a7b5dbe202..bd55c85b07 100644 --- a/test/test_mesh_beacon/test_main.cpp +++ b/test/test_mesh_beacon/test_main.cpp @@ -61,12 +61,6 @@ constexpr NodeNum kRemoteNode = 0xBBBB0002; class MockRouter : public Router { public: - ~MockRouter() - { - delete cryptLock; - cryptLock = nullptr; - } - ErrorCode send(meshtastic_MeshPacket *p) override { // Capture the primary channel as seen AT send() time. sendBeaconPacket() temporarily swaps diff --git a/test/test_mesh_module/test_main.cpp b/test/test_mesh_module/test_main.cpp index a0a23245de..208dde7bb4 100644 --- a/test/test_mesh_module/test_main.cpp +++ b/test/test_mesh_module/test_main.cpp @@ -54,12 +54,6 @@ class MockRadioInterface : public RadioInterface class MockRouter : public Router { public: - ~MockRouter() - { - delete cryptLock; - cryptLock = nullptr; - } - ErrorCode send(meshtastic_MeshPacket *p) override { sentPackets.push_back(*p); diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 06cac97dcc..aba25285c4 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -42,12 +42,6 @@ namespace class MockRouter : public Router { public: - ~MockRouter() - { - // cryptLock is created in the constructor for Router. - delete cryptLock; - cryptLock = NULL; - } void enqueueReceivedMessage(meshtastic_MeshPacket *p) override { packets_.emplace_back(*p); diff --git a/test/test_nexthop_routing/test_main.cpp b/test/test_nexthop_routing/test_main.cpp index ecfe8ac5a8..9f22602e50 100644 --- a/test/test_nexthop_routing/test_main.cpp +++ b/test/test_nexthop_routing/test_main.cpp @@ -83,17 +83,10 @@ class MockNodeDB : public NodeDB // --------------------------------------------------------------------------- // Test shim - expose getNextHop and the route-health helpers; reset health between tests. -// Nulls cryptLock so the Router base can be (re)constructed (same pattern as test_mqtt MockRouter). // --------------------------------------------------------------------------- class NextHopRouterTestShim : public NextHopRouter { public: - NextHopRouterTestShim() : NextHopRouter() - { - delete cryptLock; - cryptLock = nullptr; - } - using NextHopRouter::clearRouteHealth; using NextHopRouter::findRouteHealth; using NextHopRouter::getNextHop; diff --git a/test/test_nodeinfo_send_window/test_main.cpp b/test/test_nodeinfo_send_window/test_main.cpp index 4a8b637512..3eb5f14bd2 100644 --- a/test/test_nodeinfo_send_window/test_main.cpp +++ b/test/test_nodeinfo_send_window/test_main.cpp @@ -82,14 +82,6 @@ class MockRouter : public Router public: MockRouter() { addInterface(std::unique_ptr(new StubRadioInterface())); } - // Router's constructor asserts cryptLock is null before allocating it, so a per-test router can - // only be rebuilt if the previous one hands the global back. - ~MockRouter() - { - delete cryptLock; - cryptLock = nullptr; - } - ErrorCode send(meshtastic_MeshPacket *p) override { sentPackets.push_back(*p); diff --git a/test/test_traffic_management/test_main.cpp b/test/test_traffic_management/test_main.cpp index 086f47a51d..94d35c4e48 100644 --- a/test/test_traffic_management/test_main.cpp +++ b/test/test_traffic_management/test_main.cpp @@ -175,14 +175,6 @@ class MockRadioInterface : public RadioInterface class MockRouter : public Router { public: - ~MockRouter() - { - // Router allocates a global crypt lock in its constructor. - // Clean it up here so each test can build a fresh mock router. - delete cryptLock; - cryptLock = nullptr; - } - ErrorCode send(meshtastic_MeshPacket *p) override { sentPackets.push_back(*p);