Chasing memory corruption/leaks.

This commit is contained in:
Blaz Kristan 2023-08-07 16:50:18 +02:00
parent cb42ca8765
commit 084070475d
4 changed files with 81 additions and 59 deletions

View File

@ -5621,6 +5621,7 @@ uint16_t mode_2Dghostrider(void) {
SEGENV.aux1 = rows; SEGENV.aux1 = rows;
random16_set_seed(strip.now); random16_set_seed(strip.now);
lighter->angleSpeed = random8(0,20) - 10; lighter->angleSpeed = random8(0,20) - 10;
lighter->gAngle = random16();
lighter->Vspeed = 5; lighter->Vspeed = 5;
lighter->gPosX = (cols/2) * 10; lighter->gPosX = (cols/2) * 10;
lighter->gPosY = (rows/2) * 10; lighter->gPosY = (rows/2) * 10;
@ -5628,6 +5629,7 @@ uint16_t mode_2Dghostrider(void) {
lighter->lightersPosX[i] = lighter->gPosX; lighter->lightersPosX[i] = lighter->gPosX;
lighter->lightersPosY[i] = lighter->gPosY + i; lighter->lightersPosY[i] = lighter->gPosY + i;
lighter->time[i] = i * 2; lighter->time[i] = i * 2;
lighter->reg[i] = false;
} }
} }

View File

@ -498,14 +498,14 @@ typedef struct Segment {
Segment(Segment &&orig) noexcept; // move constructor Segment(Segment &&orig) noexcept; // move constructor
~Segment() { ~Segment() {
//#ifdef WLED_DEBUG #ifdef WLED_DEBUG
//Serial.print(F("Destroying segment:")); Serial.printf("-- Destroying segment: %p\n", this);
//if (name) Serial.printf(" %s (%p)", name, name); //if (name) Serial.printf(" %s (%p)", name, name);
//if (data) Serial.printf(" %d (%p)", (int)_dataLen, data); //if (data) Serial.printf(" %d (%p)", (int)_dataLen, data);
//Serial.println(); //Serial.println();
//#endif #endif
if (name) { delete[] name; name = nullptr; } if (name) { delete[] name; name = nullptr; }
if (_t) { transitional = false; delete _t; _t = nullptr; } stopTransition();
deallocateData(); deallocateData();
} }
@ -559,6 +559,7 @@ typedef struct Segment {
// transition functions // transition functions
void startTransition(uint16_t dur); // transition has to start before actual segment values change void startTransition(uint16_t dur); // transition has to start before actual segment values change
void stopTransition(void);
void handleTransition(void); void handleTransition(void);
void saveSegenv(tmpsegd_t *tmpSegD = nullptr); void saveSegenv(tmpsegd_t *tmpSegD = nullptr);
void restoreSegenv(tmpsegd_t *tmpSegD = nullptr); void restoreSegenv(tmpsegd_t *tmpSegD = nullptr);

View File

@ -85,7 +85,7 @@ bool Segment::_modeBlend = false;
// copy constructor // copy constructor
Segment::Segment(const Segment &orig) { Segment::Segment(const Segment &orig) {
//DEBUG_PRINTLN(F("-- Copy segment constructor --")); //DEBUG_PRINTF("-- Copy segment constructor: %p -> %p\n", &orig, this);
memcpy((void*)this, (void*)&orig, sizeof(Segment)); memcpy((void*)this, (void*)&orig, sizeof(Segment));
transitional = false; // copied segment cannot be in transition transitional = false; // copied segment cannot be in transition
name = nullptr; name = nullptr;
@ -94,12 +94,12 @@ Segment::Segment(const Segment &orig) {
_t = nullptr; _t = nullptr;
if (orig.name) { name = new char[strlen(orig.name)+1]; if (name) strcpy(name, orig.name); } if (orig.name) { name = new char[strlen(orig.name)+1]; if (name) strcpy(name, orig.name); }
if (orig.data) { if (allocateData(orig._dataLen)) memcpy(data, orig.data, orig._dataLen); } if (orig.data) { if (allocateData(orig._dataLen)) memcpy(data, orig.data, orig._dataLen); }
//if (orig._t) { _t = new Transition(orig._t->_dur, orig._t->_briT, orig._t->_cctT, orig._t->_colorT); } //if (orig._t) { _t = new Transition(orig._t->_dur); }
} }
// move constructor // move constructor
Segment::Segment(Segment &&orig) noexcept { Segment::Segment(Segment &&orig) noexcept {
//DEBUG_PRINTLN(F("-- Move segment constructor --")); //DEBUG_PRINTF("-- Move segment constructor: %p -> %p\n", &orig, this);
memcpy((void*)this, (void*)&orig, sizeof(Segment)); memcpy((void*)this, (void*)&orig, sizeof(Segment));
orig.transitional = false; // old segment cannot be in transition any more orig.transitional = false; // old segment cannot be in transition any more
orig.name = nullptr; orig.name = nullptr;
@ -110,12 +110,15 @@ Segment::Segment(Segment &&orig) noexcept {
// copy assignment // copy assignment
Segment& Segment::operator= (const Segment &orig) { Segment& Segment::operator= (const Segment &orig) {
//DEBUG_PRINTLN(F("-- Copying segment --")); //DEBUG_PRINTF("-- Copying segment: %p -> %p\n", &orig, this);
if (this != &orig) { if (this != &orig) {
// clean destination // clean destination
transitional = false; // copied segment cannot be in transition transitional = false; // copied segment cannot be in transition
if (name) delete[] name; if (name) delete[] name;
if (_t) delete _t; if (_t) {
if (_t->_tmpSeg._dataT) free(_t->_tmpSeg._dataT);
delete _t;
}
deallocateData(); deallocateData();
// copy source // copy source
memcpy((void*)this, (void*)&orig, sizeof(Segment)); memcpy((void*)this, (void*)&orig, sizeof(Segment));
@ -135,12 +138,16 @@ Segment& Segment::operator= (const Segment &orig) {
// move assignment // move assignment
Segment& Segment::operator= (Segment &&orig) noexcept { Segment& Segment::operator= (Segment &&orig) noexcept {
//DEBUG_PRINTLN(F("-- Moving segment --")); //DEBUG_PRINTF("-- Moving segment: %p -> %p\n", &orig, this);
if (this != &orig) { if (this != &orig) {
transitional = false; // just temporary transitional = false; // just temporary
if (name) { delete[] name; name = nullptr; } // free old name if (name) { delete[] name; name = nullptr; } // free old name
deallocateData(); // free old runtime data deallocateData(); // free old runtime data
if (_t) { delete _t; _t = nullptr; } if (_t) {
if (_t->_tmpSeg._dataT) free(_t->_tmpSeg._dataT);
delete _t;
_t = nullptr;
}
memcpy((void*)this, (void*)&orig, sizeof(Segment)); memcpy((void*)this, (void*)&orig, sizeof(Segment));
orig.transitional = false; // old segment cannot be in transition orig.transitional = false; // old segment cannot be in transition
orig.name = nullptr; orig.name = nullptr;
@ -153,12 +160,14 @@ Segment& Segment::operator= (Segment &&orig) noexcept {
bool Segment::allocateData(size_t len) { bool Segment::allocateData(size_t len) {
if (data && _dataLen == len) return true; //already allocated if (data && _dataLen == len) return true; //already allocated
//DEBUG_PRINTF("-- Allocating data (%d): %p\n", len, this);
deallocateData(); deallocateData();
if (Segment::getUsedSegmentData() + len > MAX_SEGMENT_DATA) return false; //not enough memory if (Segment::getUsedSegmentData() + len > MAX_SEGMENT_DATA) { DEBUG_PRINTF("!!! Effect RAM depleted: %d/%d !!!\n", len, Segment::getUsedSegmentData()); return false; } //not enough memory
// do not use SPI RAM on ESP32 since it is slow // do not use SPI RAM on ESP32 since it is slow
data = (byte*) malloc(len); data = (byte*) malloc(len);
if (!data) return false; //allocation failed if (!data) { DEBUG_PRINTLN(F("!!! Allocation failed. !!!")); return false; } //allocation failed
Segment::addUsedSegmentData(len); Segment::addUsedSegmentData(len);
DEBUG_PRINTF("--- Allocated data (%p): %d/%d -> %p\n", this, len, Segment::getUsedSegmentData(), data);
_dataLen = len; _dataLen = len;
memset(data, 0, len); memset(data, 0, len);
return true; return true;
@ -166,9 +175,11 @@ bool Segment::allocateData(size_t len) {
void Segment::deallocateData() { void Segment::deallocateData() {
if (!data) return; if (!data) return;
DEBUG_PRINTF("--- Released data (%d/%d): %p -> %p\n", _dataLen, Segment::getUsedSegmentData(), this, data);
free(data); free(data);
data = nullptr; data = nullptr;
Segment::addUsedSegmentData(-_dataLen); // WARNING it looks like we have a memory leak somewhere
Segment::addUsedSegmentData(_dataLen <= Segment::getUsedSegmentData() ? -_dataLen : -Segment::getUsedSegmentData());
_dataLen = 0; _dataLen = 0;
} }
@ -181,7 +192,7 @@ void Segment::deallocateData() {
*/ */
void Segment::resetIfRequired() { void Segment::resetIfRequired() {
if (!reset) return; if (!reset) return;
DEBUG_PRINTLN(F("-- Segment reset.")); //DEBUG_PRINTF("-- Segment reset: %p\n", this);
startTransition(0); // stop pending transition startTransition(0); // stop pending transition
deallocateData(); deallocateData();
next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0; next_time = 0; step = 0; call = 0; aux0 = 0; aux1 = 0;
@ -273,15 +284,8 @@ CRGBPalette16 &Segment::loadPalette(CRGBPalette16 &targetPalette, uint8_t pal) {
void Segment::startTransition(uint16_t dur) { void Segment::startTransition(uint16_t dur) {
if (!dur) { if (!dur) {
transitional = false; if (_t) _t->_dur = dur; // this will stop transition in next handleTransisiton()
if (_t) { else transitional = false;
if (_t->_tmpSeg._dataT && _t->_tmpSeg._dataLenT > 0) {
free(_t->_tmpSeg._dataT);
_t->_tmpSeg._dataT = nullptr;
}
delete _t;
_t = nullptr;
}
return; return;
} }
if (transitional && _t) return; // already in transition no need to store anything if (transitional && _t) return; // already in transition no need to store anything
@ -290,18 +294,20 @@ void Segment::startTransition(uint16_t dur) {
_t = new Transition(dur); // no previous transition running _t = new Transition(dur); // no previous transition running
if (!_t) return; // failed to allocate data if (!_t) return; // failed to allocate data
//DEBUG_PRINTF("-- Started transition: %p\n", this);
saveSegenv(); saveSegenv();
CRGBPalette16 _palT = CRGBPalette16(DEFAULT_COLOR); loadPalette(_palT, palette); CRGBPalette16 _palT = CRGBPalette16(DEFAULT_COLOR); loadPalette(_palT, palette);
_t->_palT = _palT; _t->_palT = _palT;
_t->_modeT = mode; _t->_modeT = mode;
_t->_briT = on ? opacity : 0; _t->_briT = on ? opacity : 0;
_t->_cctT = cct; _t->_cctT = cct;
_t->_tmpSeg._optionsT |= 0b0000000001000000; // mark old segment transitional
_t->_tmpSeg._dataLenT = 0; _t->_tmpSeg._dataLenT = 0;
_t->_tmpSeg._dataT = nullptr; _t->_tmpSeg._dataT = nullptr;
if (_dataLen > 0 && data) { if (_dataLen > 0 && data) {
_t->_tmpSeg._dataT = (byte *)malloc(_dataLen); _t->_tmpSeg._dataT = (byte *)malloc(_dataLen);
if (_t->_tmpSeg._dataT) { if (_t->_tmpSeg._dataT) {
DEBUG_PRINTLN(F("-- Allocated duplicate data.")); //DEBUG_PRINTF("-- Allocated duplicate data (%d): %p\n", _dataLen, _t->_tmpSeg._dataT);
memcpy(_t->_tmpSeg._dataT, data, _dataLen); memcpy(_t->_tmpSeg._dataT, data, _dataLen);
_t->_tmpSeg._dataLenT = _dataLen; _t->_tmpSeg._dataLenT = _dataLen;
} }
@ -309,6 +315,27 @@ void Segment::startTransition(uint16_t dur) {
transitional = true; // setOption(SEG_OPTION_TRANSITIONAL, true); transitional = true; // setOption(SEG_OPTION_TRANSITIONAL, true);
} }
void Segment::stopTransition() {
if (!transitional) return;
transitional = false; // finish transitioning segment
//DEBUG_PRINTF("-- Stopping transition: %p\n", this);
if (_t) {
if (_t->_tmpSeg._dataT && _t->_tmpSeg._dataLenT > 0) {
//DEBUG_PRINTF("-- Released duplicate data (%d): %p\n", _t->_tmpSeg._dataLenT, _t->_tmpSeg._dataT);
free(_t->_tmpSeg._dataT);
_t->_tmpSeg._dataT = nullptr;
}
delete _t;
_t = nullptr;
}
}
void Segment::handleTransition() {
if (!transitional) return;
uint16_t _progress = progress();
if (_progress == 0xFFFFU) stopTransition();
}
// transition progression between 0-65535 // transition progression between 0-65535
uint16_t Segment::progress() { uint16_t Segment::progress() {
if (transitional && _t) { if (transitional && _t) {
@ -318,22 +345,8 @@ uint16_t Segment::progress() {
return 0xFFFFU; return 0xFFFFU;
} }
uint8_t Segment::currentBri(uint8_t briNew, bool useCct) {
uint32_t prog = progress();
if (prog < 0xFFFFU) {
if (useCct) return ((briNew * prog) + _t->_cctT * (0xFFFFU - prog)) >> 16;
else return ((briNew * prog) + _t->_briT * (0xFFFFU - prog)) >> 16;
}
return briNew;
}
uint8_t Segment::currentMode(uint8_t newMode) {
uint16_t prog = progress(); // implicit check for transitional & _t in progress()
if (prog < 0xFFFFU) return _t->_modeT;
return newMode;
}
void Segment::saveSegenv(tmpsegd_t *tmpSeg) { void Segment::saveSegenv(tmpsegd_t *tmpSeg) {
//DEBUG_PRINTF("-- Saving temp seg: %p (%p)\n", this, tmpSeg);
if (tmpSeg == nullptr) { if (_t) tmpSeg = &(_t->_tmpSeg); else return; } if (tmpSeg == nullptr) { if (_t) tmpSeg = &(_t->_tmpSeg); else return; }
tmpSeg->_optionsT = options; tmpSeg->_optionsT = options;
for (size_t i=0; i<NUM_COLORS; i++) tmpSeg->_colorT[i] = colors[i]; for (size_t i=0; i<NUM_COLORS; i++) tmpSeg->_colorT[i] = colors[i];
@ -351,9 +364,11 @@ void Segment::saveSegenv(tmpsegd_t *tmpSeg) {
tmpSeg->_callT = call; tmpSeg->_callT = call;
tmpSeg->_dataT = data; tmpSeg->_dataT = data;
tmpSeg->_dataLenT = _dataLen; tmpSeg->_dataLenT = _dataLen;
//DEBUG_PRINTF("-- temp seg data: %p (%d,%p)\n", this, _dataLen, data);
} }
void Segment::restoreSegenv(tmpsegd_t *tmpSeg) { void Segment::restoreSegenv(tmpsegd_t *tmpSeg) {
//DEBUG_PRINTF("-- Restoring temp seg: %p (%p)\n", this, tmpSeg);
if (tmpSeg == nullptr) { if (tmpSeg == nullptr) {
if (_t) tmpSeg = &(_t->_tmpSeg); if (_t) tmpSeg = &(_t->_tmpSeg);
else return; else return;
@ -364,6 +379,9 @@ void Segment::restoreSegenv(tmpsegd_t *tmpSeg) {
_t->_tmpSeg._aux1T = aux1; _t->_tmpSeg._aux1T = aux1;
_t->_tmpSeg._stepT = step; _t->_tmpSeg._stepT = step;
_t->_tmpSeg._callT = call; _t->_tmpSeg._callT = call;
if (_t->_tmpSeg._dataT != data) DEBUG_PRINTF("--- data re-allocated: (%p) %p -> %p\n", this, _t->_tmpSeg._dataT, data);
_t->_tmpSeg._dataT = data; // sometimes memory gets re-allocated (!! INVESTIGATE WHY !!)
_t->_tmpSeg._dataLenT = _dataLen; // sometimes memory gets re-allocated (!! INVESTIGATE WHY !!)
} }
} }
options = tmpSeg->_optionsT; options = tmpSeg->_optionsT;
@ -382,6 +400,22 @@ void Segment::restoreSegenv(tmpsegd_t *tmpSeg) {
call = tmpSeg->_callT; call = tmpSeg->_callT;
data = tmpSeg->_dataT; data = tmpSeg->_dataT;
_dataLen = tmpSeg->_dataLenT; _dataLen = tmpSeg->_dataLenT;
//DEBUG_PRINTF("-- temp seg data: %p (%d,%p)\n", this, _dataLen, data);
}
uint8_t Segment::currentBri(uint8_t briNew, bool useCct) {
uint32_t prog = progress();
if (prog < 0xFFFFU) {
if (useCct) return ((briNew * prog) + _t->_cctT * (0xFFFFU - prog)) >> 16;
else return ((briNew * prog) + _t->_briT * (0xFFFFU - prog)) >> 16;
}
return briNew;
}
uint8_t Segment::currentMode(uint8_t newMode) {
uint16_t prog = progress(); // implicit check for transitional & _t in progress()
if (prog < 0xFFFFU) return _t->_modeT;
return newMode;
} }
uint32_t Segment::currentColor(uint8_t slot, uint32_t colorNew) { uint32_t Segment::currentColor(uint8_t slot, uint32_t colorNew) {
@ -402,24 +436,6 @@ CRGBPalette16 &Segment::currentPalette(CRGBPalette16 &targetPalette, uint8_t pal
return targetPalette; return targetPalette;
} }
void Segment::handleTransition() {
if (!transitional) return;
uint16_t _progress = progress();
if (_progress == 0xFFFFU) transitional = false; // finish transitioning segment
if (_t) { // thanks to @nXm AKA https://github.com/NMeirer
//if (_progress >= 32767U && _t->_modeP != mode) markForReset();
if (_progress == 0xFFFFU) {
DEBUG_PRINTLN(F("-- Stopping transition."));
if (_t->_tmpSeg._dataT && _t->_tmpSeg._dataLenT > 0) {
free(_t->_tmpSeg._dataT);
_t->_tmpSeg._dataT = nullptr;
}
delete _t;
_t = nullptr;
}
}
}
// relies on WS2812FX::service() to call it max every 8ms or more (MIN_SHOW_DELAY) // relies on WS2812FX::service() to call it max every 8ms or more (MIN_SHOW_DELAY)
void Segment::handleRandomPalette() { void Segment::handleRandomPalette() {
// just do a blend; if the palettes are identical it will just compare 48 bytes (same as _randomPalette == _newRandomPalette) // just do a blend; if the palettes are identical it will just compare 48 bytes (same as _randomPalette == _newRandomPalette)
@ -1186,7 +1202,7 @@ void WS2812FX::service() {
Segment::tmpsegd_t _tmpSegData; Segment::tmpsegd_t _tmpSegData;
seg.saveSegenv(&_tmpSegData); seg.saveSegenv(&_tmpSegData);
uint8_t tmpMode = seg.currentMode(seg.mode); uint8_t tmpMode = seg.currentMode(seg.mode);
if (seg.mode != tmpMode) seg.restoreSegenv(); // restore transition data (including temporary opacity) if (seg.mode != tmpMode) seg.restoreSegenv(); // restore transition data
delay = (*_mode[tmpMode])(); // run old mode delay = (*_mode[tmpMode])(); // run old mode
if (seg.mode != tmpMode) { if (seg.mode != tmpMode) {
if (tmpMode != FX_MODE_HALLOWEEN_EYES) seg.call++; if (tmpMode != FX_MODE_HALLOWEEN_EYES) seg.call++;

View File

@ -29,8 +29,11 @@ bool deserializeSegment(JsonObject elem, byte it, byte presetId)
id = strip.getSegmentsNum()-1; // segments are added at the end of list id = strip.getSegmentsNum()-1; // segments are added at the end of list
} }
//DEBUG_PRINTLN("-- JSON deserialize segment.");
Segment& seg = strip.getSegment(id); Segment& seg = strip.getSegment(id);
//DEBUG_PRINTF("-- Original segment: %p\n", &seg);
Segment prev = seg; //make a backup so we can tell if something changed Segment prev = seg; //make a backup so we can tell if something changed
//DEBUG_PRINTF("-- Duplicate segment: %p\n", &prev);
uint16_t start = elem["start"] | seg.start; uint16_t start = elem["start"] | seg.start;
if (stop < 0) { if (stop < 0) {