fix(NextHopRouter): don't use the pending record after a send re-enters the router

[h3lix1]

doRetransmissions() holds a reference into the pending map across calls that can delete the entry
it is iterating, then keeps using it.

The re-entry is synchronous, which is the part that makes this reachable rather than theoretical.
Both sendAckNak() on the MAX_RETRANSMIT path and the sends below it reach Router::send(), whose
duty-cycle and encode-failure paths call abortSendAndNak() -> sendLocal() -> deliverLocal().
deliverLocal() only defers the loopback when handleDepth > 0, and doRetransmissions() runs from
runOnce() rather than from inside handleReceived(), so handleDepth is 0 and the delivery
happens inline on this stack. The NAK is addressed to us with request_id set to the packet being
retried, so ReliableRouter::sniffReceived() calls stopRetransmission() for exactly this key, which
erases the map node and releases p.packet.

Control then returns into the loop, which writes --p.numRetransmissions, hands &p to
setNextTx() which dereferences p.packet, and reads p.nextTxMsec for the sleep calculation. On
the MAX_RETRANSMIT path it also reads it->first off the freed node to pass to
stopRetransmission().

The fix copies the key by value up front and re-looks the record up after the sends instead of
reusing the reference. Where the lookup comes back empty the record is gone and there is nothing left
to reschedule.

When it fires

The duty-cycle abort needs a region with dutyCycle < 100, so EU_433, EU_868, EU_866, EU_N_868, TH
and UA_433. US, ANZ and JP never take it.

The encode-failure abort has no such precondition. The pending copy is the decoded packet, since
ReliableRouter::send copies before Router::send encrypts, so every retry re-encodes and a channel
or PKI state change between attempts reaches the same path.

About the test

MockRoutingModule in this suite records sendAckNak() and stops, so nothing here exercised the
loopback at all. The second commit adds a routing module that re-enters the router the way the real
path does. Seeded with one attempt the record sits at numRetransmissions == 0, so the first due
pass takes the MAX_RETRANSMIT branch and the loopback deletes the entry before control returns.

Be aware of what this test does and does not catch on its own. Revert the fix and it still passes
under plain native-macos, because reading a freed unordered_map node is undefined rather than
reliably fatal. It needs an allocator that actually enforces the lifetime.

Reproduced

native-macos-debug carries -fsanitize=address but does not start on macOS 15: it spins inside
__asan::AsanInitInternal() -> InitializeShadowMemory() -> MemoryRangeIsAvailable() ->
get_dyld_hdr() -> dyld_shared_cache_iterate_text_swift, which calls malloc, which ASAN has
already intercepted while still initialising. It never reaches main.

macOS Guard Malloc works where ASAN does not, and it unmaps freed pages, so the read faults instead
of silently succeeding:

pio test -e native-macos -f test_nexthop_routing --without-testing
DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib ./.pio/build/native-macos/meshtasticd

• develop with only this PR's test file applied: SIGSEGV, exit 139, immediately after
  test_reliableAckStopsNormalPendingTransmission passes and on entry to
  test_retransmissionSurvivesLoopbackErasingItsOwnRecord.
• this branch: exit 0, 47 Tests 0 Failures 0 Ignored.

So the lifetime bug is observed, not just argued from the source. That recipe may be worth knowing
generally for this repo, since it gives macOS contributors use-after-free detection that the ASAN
environments cannot currently provide.

Hardware

Rebased onto current develop, so this now sits on top of #11320 and #11502. Flashed to two nodes, a
Heltec V4 and a Seeed XIAO ESP32-S3, both running 2.8.0.6d0f264 from this branch, and driven
through the path the bug is on.

The interesting case is a reliable unicast whose retry ladder runs out, because that is what makes
doRetransmissions() call sendAckNak(MAX_RETRANSMIT), which delivers inline and re-enters the
router to erase the very record the loop is holding. To force it I disabled lora.tx_enabled on the
XIAO so it would receive but never ACK.

• Reachable DM with want_ack: Received an ACK.
• Same DM with the peer unable to reply: ladder ran ~37 s and ended with
  Received a NAK, error reason: MAX_RETRANSMIT. Repeated three more times, same result each time.
• TX re-enabled: Received an ACK again, mesh recovered with no intervention.
• rebootCount was 13 on the Heltec and 10 on the XIAO before and after the whole sequence, so
  nothing panicked or reset.

I then flashed unfixed develop to the Heltec and ran the identical sequence, so as not to claim more
than the bench shows. It behaves the same: four MAX_RETRANSMIT NAKs, rebootCount unchanged.

So the hardware run demonstrates no regression, and nothing more. It does not discriminate between
fixed and unfixed, which is the expected outcome for a read of a freed heap block that nothing has
reallocated yet. The Guard Malloc run above is the part that actually pins the bug.

Testing: builds clean on heltec-v4 and seeed-xiao-s3, 148/148 across test_nexthop_routing,
test_packet_signing and test_mesh_module.

Note on rebasing: #11291 rewrites the if (p.nextTxMsec <= now) line this patch sits next to and
adds a CI check that rejects raw millis() comparisons. If that lands first the fix needs rebasing
onto Throttle::deadlinePassedAt, and the shape of it does not change. Worth also knowing that
#11235 edits setNextTx()'s prologue, so a fix that changed that function's signature would collide.
This one deliberately stays inside doRetransmissions().

🤖 Generated with Claude Code

🤝 Attestations

• I have tested that my proposed changes behave as described.
• I have tested that my proposed changes do not cause any obvious regressions on the following devices:
  • Heltec (Lora32) V3
  • LilyGo T-Deck
  • LilyGo T-Beam
  • RAK WisBlock 4631
  • Seeed Studio T-1000E tracker card
  • Other: Heltec V4 and Seeed XIAO ESP32-S3, two-node bench, ACK and MAX_RETRANSMIT paths both
    exercised, and the same sequence run on unfixed develop as a control

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a139f22-4c95-42f0-9550-ae233d112e44

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.