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 <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VeY5PczVYsxWNZhE2Cx4Sv
This commit is contained in:
ClaudeandJonathan Bennett committed 2026-09-26 14:21:08 +00:00
1 parent 55835e8bc6
commit 79afdeff8d
7 files changed
-46

No files matched your search

-5
View File
@@ -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);
-6
View File
@@ -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
-6
View File
@@ -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);
-6
View File
@@ -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);
-7
View File
@@ -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;
@@ -82,14 +82,6 @@ class MockRouter : public Router
public:
MockRouter() { addInterface(std::unique_ptr<RadioInterface>(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);
@@ -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);