Make the frame-end anchor atomic

The radio worker writes lastFrameEndMs from onNotify() while a draw for a
packet being queued reads it on whatever thread called send(), so the access
was an unsynchronized read of a plain uint32_t. std::atomic with relaxed
ordering is free on every target here and matches MeshService's fromNum.

Relaxed is the right ordering: the anchor publishes nothing but itself, both
writers are the radio worker so the compare-then-store races with no other
write, and losing the race against a reader only anchors that one draw to the
frame before last.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
This commit is contained in:
Jonathan BennettandClaude Opus 5 committed 2026-10-09 16:46:57 +00:00
1 parent dfb89cc81e
commit 21ded4e7ed
3 files changed
+15 -7

No files matched your search

+7 -4
View File
@@ -837,9 +837,11 @@ uint32_t RadioInterface::getTxDelayMsec(const meshtastic_MeshPacket *p)
void RadioInterface::noteFrameEnd(uint32_t endMs, const char *what)
{
if (lastFrameEndMs && (int32_t)(endMs - lastFrameEndMs) < 0)
// Both writers are the radio worker, so this read-modify-write races with no other write.
const uint32_t held = lastFrameEndMs.load(std::memory_order_relaxed);
if (held && (int32_t)(endMs - held) < 0)
return; // a frame handed to us late, after one that ended later
lastFrameEndMs = Time::skipZero(endMs);
lastFrameEndMs.store(Time::skipZero(endMs), std::memory_order_relaxed);
lastFrameEndWhat = what;
}
@@ -864,14 +866,15 @@ uint32_t RadioInterface::getAnchoredSlotDelayMsec(uint32_t slots, meshtastic_Mes
// Two nodes that redraw after the same frame count their slots from its air end rather than from
// their own handling of it, so their grids line up however differently they got there. Taking
// only one parity's slots then rules out the same slot, so the two are always a slot time apart.
const uint32_t sinceEnd = lastFrameEndMs ? Time::getMillis() - lastFrameEndMs : 0;
const uint32_t frameEnd = lastFrameEndMs.load(std::memory_order_relaxed);
const uint32_t sinceEnd = frameEnd ? Time::getMillis() - frameEnd : 0;
const uint32_t ownParitySlots = slots / 2 ? slots / 2 : 1;
const uint32_t delay = anchoredSlotDelayMsec(sinceEnd, slotTimeMsec, random(0, ownParitySlots), parity);
LOG_TRACE("TX slot anchor: %u ms into slot %u of %u, parity %u, %u ms after %s end", (unsigned)delay,
(unsigned)((sinceEnd + delay) / slotTimeMsec), (unsigned)slots,
(unsigned)(parity == meshtastic_MeshPacket_SlotParity_SLOT_PARITY_ODD ? 1 : 0), (unsigned)sinceEnd,
lastFrameEndMs ? lastFrameEndWhat : "no");
frameEnd ? lastFrameEndWhat : "no");
return delay;
}
+7 -2
View File
@@ -6,6 +6,7 @@
#include "PointerQueue.h"
#include "airtime.h"
#include "error.h"
#include <atomic>
#include <memory>
#if HAS_LORA_FEM
@@ -111,8 +112,12 @@ class RadioInterface
/** millis() when the last frame this node sent or heard left the air, 0 before the first.
* Only read by a draw that was asked for a slot parity; the driver keeps it up to date so any
* module can ask at any time. */
uint32_t lastFrameEndMs = 0;
* module can ask at any time.
*
* Atomic because the radio worker writes it from onNotify() while a draw for a packet being
* queued reads it on whatever thread called send(). Relaxed is enough: it publishes nothing
* but itself, and losing the race just anchors that one draw to the frame before last. */
std::atomic<uint32_t> lastFrameEndMs{0};
/** "tx" or "rx" for the frame lastFrameEndMs came from, for the trace line */
const char *lastFrameEndWhat = "none";
uint16_t preambleLength = 16; // 8 is default, but we use longer to increase the amount of sleep time when receiving
+1 -1
View File
@@ -80,7 +80,7 @@ class TestableRadioInterface : public RadioInterface
size_t beginSendingPublic(meshtastic_MeshPacket *p) { return beginSending(p); }
meshtastic_MeshPacket *getSendingPacket() const { return sendingPacket; }
void clearSendingPacketForTest() { sendingPacket = nullptr; }
uint32_t getLastFrameEndMs() const { return lastFrameEndMs; }
uint32_t getLastFrameEndMs() const { return lastFrameEndMs.load(std::memory_order_relaxed); }
// Override reconfigure to call the base which invokes applyModemConfig()
bool reconfigure() override { return RadioInterface::reconfigure(); }