From 188956a4af844cdec15c274892e2914f10bf0e77 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Fri, 8 Sep 2023 12:06:23 +0200 Subject: [PATCH] bugfix for random crash when switching between presets this is a band-aid fix for random crashes when switching between presets with multiple segments - crossfade disabled. !! adding type initializers fixed it for me on -S3, however I still see (less frequent) crashes on esp32, due to heap corruption. It took me hours to get a meaningful stackdump: assert failed: heap_caps_free heap_caps.c:360 (heap != NULL && "free() target pointer is outside heap areas") Backtrace: 0x40084ee1:0x3ffb2570 0x4008e341:0x3ffb2590 0x40094709:0x3ffb25b0 0x4008534a:0x3ffb26e0 0x40094739:0x3ffb2700 0x400e9037:0x3ffb2720 0x400e917c:0x3ffb2740 0x400eaeeb:0x3ffb2760 0x40117ec5:0x3ffb27c0 0x401184ea:0x3ffb2800 0x4013509d:0x3ffb2820 #0 0x40084ee1:0x3ffb2570 in panic_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/panic.c:402 #1 0x4008e341:0x3ffb2590 in esp_system_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/esp_system.c:128 #2 0x40094709:0x3ffb25b0 in __assert_func at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/newlib/assert.c:85 #3 0x4008534a:0x3ffb26e0 in heap_caps_free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/heap/heap_caps.c:360 (inlined by) heap_caps_free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/heap/heap_caps.c:345 #4 0x40094739:0x3ffb2700 in free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/newlib/heap.c:39 #5 0x400e9037:0x3ffb2720 in Segment::deallocateData() at wled00/FX_fcn.cpp:189 #6 0x400e917c:0x3ffb2740 in Segment::resetIfRequired() at wled00/FX_fcn.cpp:206 (inlined by) Segment::resetIfRequired() at wled00/FX_fcn.cpp:203 #7 0x400eaeeb:0x3ffb2760 in WS2812FX::service() at wled00/FX_fcn.cpp:1216 (discriminator 2) #8 0x40117ec5:0x3ffb27c0 in WLED::loop() at wled00/wled.cpp:115 (discriminator 3) #9 0x401184ea:0x3ffb2800 in loop() at C:/src/wled00/wled00.ino:20 #10 0x4013509d:0x3ffb2820 in loopTask(void*) at C:/Users/user/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:50 ELF file SHA256: 18c20b536f4c6ef4 --- wled00/FX.h | 8 ++++---- wled00/FX_fcn.cpp | 12 ++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/wled00/FX.h b/wled00/FX.h index d7eebc4f..28410783 100644 --- a/wled00/FX.h +++ b/wled00/FX.h @@ -395,10 +395,10 @@ typedef struct Segment { }; uint16_t _aux0T; uint16_t _aux1T; - uint32_t _stepT; - uint32_t _callT; - uint8_t *_dataT; - uint16_t _dataLenT; + uint32_t _stepT = 0; + uint32_t _callT = 0; + uint8_t *_dataT = nullptr; + uint16_t _dataLenT = 0; } tmpsegd_t; private: diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index f31b1899..9a3a4ded 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -168,6 +168,7 @@ bool Segment::allocateData(size_t len) { if (data && _dataLen == len) return true; //already allocated //DEBUG_PRINTF("-- Allocating data (%d): %p\n", len, this); deallocateData(); + if (len == 0) return(false); // nothing to do if (Segment::getUsedSegmentData() + len > MAX_SEGMENT_DATA) { // not enough memory DEBUG_PRINTF("!!! Effect RAM depleted: %d/%d !!!\n", len, Segment::getUsedSegmentData()); @@ -184,9 +185,13 @@ bool Segment::allocateData(size_t len) { } void Segment::deallocateData() { - if (!data) return; + if (!data) { _dataLen = 0; return; } //DEBUG_PRINTF("--- Released data (%p): %d/%d -> %p\n", this, _dataLen, Segment::getUsedSegmentData(), data); - free(data); + if ((Segment::getUsedSegmentData() > 0) && (_dataLen > 0)) { // check that we don't have a dangling / inconsistent data pointer + free(data); + } else { + DEBUG_PRINTF("---- Released data (%p): inconsistent UsedSegmentData (%d/%d), cowardly refusing to free nothing.\n", this, _dataLen, Segment::getUsedSegmentData()); + } data = nullptr; // WARNING it looks like we have a memory leak somewhere Segment::addUsedSegmentData(_dataLen <= Segment::getUsedSegmentData() ? -_dataLen : -Segment::getUsedSegmentData()); @@ -338,6 +343,7 @@ void Segment::stopTransition() { //DEBUG_PRINTF("-- Released duplicate data (%d): %p\n", _t->_segT._dataLenT, _t->_segT._dataT); free(_t->_segT._dataT); _t->_segT._dataT = nullptr; + _t->_segT._dataLenT = 0; } #endif delete _t; @@ -364,6 +370,7 @@ uint16_t Segment::progress() { void Segment::swapSegenv(tmpsegd_t &tmpSeg) { if (!_t) return; //DEBUG_PRINTF("-- Saving temp seg: %p (%p)\n", this, tmpSeg); + if (nullptr == &tmpSeg) { DEBUG_PRINTLN(F("swapSegenv(): tmpSeg is nullptr !!")); return; } // sanity check tmpSeg._optionsT = options; for (size_t i=0; i_segT) != &tmpSeg) { // update possibly changed variables to keep old effect running correctly _t->_segT._aux0T = aux0;