Guard the deferred local queue and depth counter

Author: caveman99

src/mesh/Router.cpp

@@ -1224,6 +1224,7 @@ NodeNum Router::getNodeNum()
 
 bool Router::enqueueDeferredLocal(meshtastic_MeshPacket *p, RxSource src)
 {
+    concurrency::LockGuard g(&deferredLock);
     if (deferredLocalCount >= deferredLocalCapacity)
         return false;
     uint8_t tail = (deferredLocalHead + deferredLocalCount) % deferredLocalCapacity;
@@ -1235,6 +1236,7 @@ bool Router::enqueueDeferredLocal(meshtastic_MeshPacket *p, RxSource src)
 
 bool Router::dequeueDeferredLocal(DeferredLocal &out)
 {
+    concurrency::LockGuard g(&deferredLock);
     if (deferredLocalCount == 0)
         return false;
     out = deferredLocalQueue[deferredLocalHead];
@@ -1246,7 +1248,12 @@ bool Router::dequeueDeferredLocal(DeferredLocal &out)
 void Router::deliverLocal(meshtastic_MeshPacket *p, RxSource src)
 {
     // Top level: handle synchronously, exactly as before the depth guard existed.
-    if (handleDepth == 0) {
+    bool nested;
+    {
+        concurrency::LockGuard g(&deferredLock);
+        nested = handleDepth > 0;
+    }
+    if (!nested) {
         handleReceived(p, src);
         return;
     }
@@ -1275,29 +1282,36 @@ void Router::deliverLocal(meshtastic_MeshPacket *p, RxSource src)
  */
 void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
 {
-    handleDepth++;
+    {
+        concurrency::LockGuard g(&deferredLock);
+        handleDepth++;
 #ifdef PIO_UNIT_TESTING
-    if (handleDepth > maxHandleDepthObserved)
-        maxHandleDepthObserved = handleDepth;
+        if (handleDepth > maxHandleDepthObserved)
+            maxHandleDepthObserved = handleDepth;
 #endif
+    }
 
     dispatchReceived(p, src);
 
-    // Only the outermost frame drains. Deferred packets were produced by modules sending from
-    // inside dispatchReceived()'s callModules(); process them here, after the triggering frame has
-    // unwound, so a second handleReceived() never sits on top of a module handler. handleDepth
-    // stays >= 1 through the drain, so a drained packet whose own modules send more loopback
-    // packets enqueues them for this same loop rather than recursing: the stack stays flat and
-    // processing is breadth-first.
-    if (handleDepth == 1) {
+    // Only the outermost frame drains, after the triggering frame unwound, so a second
+    // handleReceived() never sits on a module handler. Depth stays >=1, keeping the drain flat.
+    bool outermost;
+    {
+        concurrency::LockGuard g(&deferredLock);
+        outermost = handleDepth == 1;
+    }
+    if (outermost) {
         DeferredLocal d;
         while (dequeueDeferredLocal(d)) {
             dispatchReceived(d.p, d.src);
             packetPool.release(d.p);
         }
     }
 
-    handleDepth--;
+    {
+        concurrency::LockGuard g(&deferredLock);
+        handleDepth--;
+    }
 }
 
 void Router::dispatchReceived(meshtastic_MeshPacket *p, RxSource src)

src/mesh/Router.h

@@ -7,6 +7,7 @@
 #include "PacketHistory.h"
 #include "PointerQueue.h"
 #include "RadioInterface.h"
+#include "concurrency/LockGuard.h"
 #include "concurrency/OSThread.h"
 #include <memory>
 
@@ -192,6 +193,10 @@ class Router : protected concurrency::OSThread, protected PacketHistory
     /// so a locally-sent loopback packet must be deferred rather than handled synchronously.
     uint8_t handleDepth = 0;
 
+    /// Guards handleDepth and the deferred ring below. nRF52 drives the router from the BLE task as
+    /// well as the loop task, so both are read-modify-written from two tasks.
+    concurrency::Lock deferredLock;
+
     /// A local loopback packet whose handleReceived() was deferred because it was produced from
     /// inside callModules(). The queue owns the packet; its RxSource travels with it so the drain
     /// dispatches it with the origin the sender intended (RX_SRC_LOCAL stays local).