fix(mesh): don't assert on malloc() failure in MemoryDynamic::alloc() (#11197)
* fix(mesh): don't assert on malloc() failure in MemoryDynamic::alloc() MemoryDynamic<T>::alloc() called assert(p) right after malloc(), instead of returning nullptr like the static MemoryPool<T,N>::alloc() already does on exhaustion. All of packetPool's callers were already hardened to null-check allocCopy()/allocZeroed() (#10948, #10951), but that path is unreachable on real OOM here: assert() fires first, one level down, before the caller ever gets a chance to check anything. On STM32WL (packetPool is MemoryDynamic there - not enough static RAM for the fixed pool), assert() failures are wrapped to an infinite `while(true);` loop rather than aborting or resetting, so the thread just hangs forever instead of returning nullptr. Reproduced on wio-e5 hardware under a burst of incoming DMs, with free heap draining toward zero right before the hang. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix: null-check the two allocUnique*() call sites missed by prior audits #10948/#10951 null-checked every raw-pointer packetPool.allocCopy()/ allocZeroed() call site, but missed the two spots using the UniquePacketPoolPacket (unique_ptr) wrapper: UdpMulticastHandler::onReceive() and MQTT::onReceive() both dereferenced the allocation result unconditionally. Previously unreachable in practice: MemoryDynamic::alloc() asserted before ever returning null, so nothing downstream saw it. Fixed alongside that assert removal (this branch) since these two are now reachable with a genuine null - same fix as everywhere else, just an unchecked pointer that turns up under OOM. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix(SimRadio): don't leave isReceiving stuck true on allocation failure startReceive() set isReceiving = true before allocCopy(), so a failed allocation (now reachable with a real nullptr instead of hanging in assert()) left the simulated radio permanently marked as receiving: handleReceiveInterrupt() returns immediately on a null receivingPacket, before ever reaching the code that would clear isReceiving. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
This commit is contained in:
co-authored by
GitHub
Ben Meadors
parent
3f4e7cc622
commit
abf217c38d
@@ -114,7 +114,10 @@ template <class T> class MemoryDynamic : public Allocator<T>
|
|||||||
virtual T *alloc(TickType_t maxWait) override
|
virtual T *alloc(TickType_t maxWait) override
|
||||||
{
|
{
|
||||||
T *p = (T *)malloc(sizeof(T));
|
T *p = (T *)malloc(sizeof(T));
|
||||||
assert(p);
|
if (!p) {
|
||||||
|
LOG_WARN("malloc(%u) failed, heap exhausted!", (unsigned)sizeof(T));
|
||||||
|
return nullptr;
|
||||||
|
}
|
||||||
this->auditAdd((int32_t)sizeof(T));
|
this->auditAdd((int32_t)sizeof(T));
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -84,6 +84,8 @@ class UdpMulticastHandler final
|
|||||||
mp.pki_encrypted = false;
|
mp.pki_encrypted = false;
|
||||||
mp.public_key.size = 0;
|
mp.public_key.size = 0;
|
||||||
UniquePacketPoolPacket p = packetPool.allocUniqueCopy(mp);
|
UniquePacketPoolPacket p = packetPool.allocUniqueCopy(mp);
|
||||||
|
if (!p)
|
||||||
|
return;
|
||||||
// Unset received SNR/RSSI
|
// Unset received SNR/RSSI
|
||||||
p->rx_snr = 0;
|
p->rx_snr = 0;
|
||||||
p->rx_rssi = 0;
|
p->rx_rssi = 0;
|
||||||
|
|||||||
@@ -143,6 +143,8 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length)
|
|||||||
}
|
}
|
||||||
|
|
||||||
UniquePacketPoolPacket p = packetPool.allocUniqueZeroed();
|
UniquePacketPoolPacket p = packetPool.allocUniqueZeroed();
|
||||||
|
if (!p)
|
||||||
|
return;
|
||||||
p->from = e.packet->from;
|
p->from = e.packet->from;
|
||||||
p->to = e.packet->to;
|
p->to = e.packet->to;
|
||||||
p->id = e.packet->id;
|
p->id = e.packet->id;
|
||||||
|
|||||||
@@ -323,13 +323,19 @@ void SimRadio::startReceive(meshtastic_MeshPacket *p)
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
isReceiving = true;
|
|
||||||
receivingPacket = packetPool.allocCopy(*p);
|
receivingPacket = packetPool.allocCopy(*p);
|
||||||
|
if (!receivingPacket) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
isReceiving = true;
|
||||||
uint32_t airtimeMsec = getPacketTime(p, true);
|
uint32_t airtimeMsec = getPacketTime(p, true);
|
||||||
notifyLater(airtimeMsec, ISR_RX, false); // Model the time it is busy receiving
|
notifyLater(airtimeMsec, ISR_RX, false); // Model the time it is busy receiving
|
||||||
#else
|
#else
|
||||||
isReceiving = true;
|
|
||||||
receivingPacket = packetPool.allocCopy(*p);
|
receivingPacket = packetPool.allocCopy(*p);
|
||||||
|
if (!receivingPacket) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
isReceiving = true;
|
||||||
handleReceiveInterrupt(); // Simulate receiving the packet immediately
|
handleReceiveInterrupt(); // Simulate receiving the packet immediately
|
||||||
startTransmitTimer();
|
startTransmitTimer();
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
Reference in New Issue
Block a user