You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Updater::begin picks the 4 KB buffer by total free heap, and does not fall back to 256 B when the allocation fails #9342
Total free heap says nothing about the largest free block. On a fragmented heap the check can pass while a 4096 byte block does not exist. Then new fails and begin() returns false with UPDATE_ERROR_OOM. The 256 byte buffer is never tried.
What I measured
Core 3.1.2, ESP8266 (GeekMagic SmallTV-Ultra).
ESP.getFreeHeap() = 19.6 KB.
ESP.getMaxFreeBlockSize() = 3,888 bytes.
What I did not measure
I did not run the upload in this state. I restarted the device first. That the upload would be refused with UPDATE_ERROR_OOM comes from reading the code above. 19.6 KB is more than 8192, so the 4 KB branch is taken. A 4096 byte allocation cannot fit in a 3,888 byte block.
Why a smaller buffer should be safe
_bufferSize is private and is only used inside Updater.cpp and the Updater.h stream template. The 256 byte case already exists for low free heap (commit 01e1c58, "Updater: if low on heap memory, use smaller write buffer"). _writeBuffer() erases a sector when _currentAddress % FLASH_SECTOR_SIZE == 0, which still works with 256 byte writes.
Proposed small change (sketch, untested)
- if (ESP.getFreeHeap() > 2 * FLASH_SECTOR_SIZE) {+ if (ESP.getMaxFreeBlockSize() >= 2 * FLASH_SECTOR_SIZE) {
_bufferSize = FLASH_SECTOR_SIZE;
} else {
_bufferSize = 256;
}
_buffer = new (std::nothrow) uint8_t[_bufferSize];
+ if (!_buffer && _bufferSize > 256) {+ _bufferSize = 256;+ _buffer = new (std::nothrow) uint8_t[_bufferSize];+ }
if (!_buffer) {
The first part picks by largest block. The second part is a safety net if the allocation still fails. Either part alone would already fix the case above. I can prepare a PR if this direction is fine.
Why extra checks though. Could we just loop over uint32_t sizes[] { FLASH_SECTOR_SIZE, 256 }; and just attempt new uint8_t[sizes[N]]? It already walks through the same structures and checks max free block internally when issuing malloc calls.
Agreed, that is simpler and covers the same case: new (std::nothrow) already fails when no block of that size exists, so the extra check adds nothing. I'll open a PR with the loop (try FLASH_SECTOR_SIZE, then 256), keeping the existing low-heap behaviour (start at 256 when 8 KB or less is free, so the buffer doesn't take the last of the heap).
Activity
mcspr commentedon Nov 8, 2026
Why extra checks though. Could we just loop over
uint32_t sizes[] { FLASH_SECTOR_SIZE, 256 };and just attemptnew uint8_t[sizes[N]]? It already walks through the same structures and checks max free block internally when issuing malloc calls.sanerdemirel commentedon Nov 8, 2026
Agreed, that is simpler and covers the same case:
new (std::nothrow)already fails when no block of that size exists, so the extra check adds nothing. I'll open a PR with the loop (tryFLASH_SECTOR_SIZE, then 256), keeping the existing low-heap behaviour (start at 256 when 8 KB or less is free, so the buffer doesn't take the last of the heap).