malloc() fails when allocating ESP.getMaxFreeBlockSize() bytes

[dplasa]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12]
• Core Version: [2.7.1=20701000]
• Development Env: [Arduino IDE]
• Operating System: [Ubuntu]

Settings in IDE

• Module: [Wemos D1 mini r2]
• Flash Mode: [qio|dio|other]
• Flash Size: [4MB]
• lwip Variant: [v2 Lower Memory]
• CPU Frequency: [80Mhz]
• Upload Using: [SERIAL]
• Upload Speed: [115200] (serial upload only)

Problem Description

malloc() returns a nullpointer when allocation more than ESP.getMaxFreeBlockSize() - 16 Bytes, whereas the documentation says "ESP.getMaxFreeBlockSize() returns the maximum allocatable ram block regarding heap fragmentation".
Either the documentation should be updated or ESP.getMaxFreeBlockSize() should return a lesser size.

MCVE Sketch

#include <ESP8266WiFi.h>

void setup() {
  Serial.begin(74880);
  WiFi.mode(WIFI_OFF);

  uint16_t maxBlock = ESP.getMaxFreeBlockSize();
  uint32_t freeHeap = ESP.getFreeHeap();

  Serial.printf("free: %5ld - max: %5d\n", freeHeap, maxBlock);

  void* p = NULL;
  while (p == NULL)
  {
    p = malloc(maxBlock);
    Serial.printf("malloc(%u) = %p\n", maxBlock, p);
    --maxBlock;
  }
}
void loop() {}

Debug Messages

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 3456, room 16 
tail 0
chksum 0x84
csum 0x84
va5432625
~ld

SDK:2.2.2-dev(38a443e)/Core:2.7.1=20701000/lwIP:STABLE-2_1_2_RELEASE/glue:1.2-30-g92add50/BearSSL:5c771be
free: 53840 - max: 53832
malloc(53832) = 0
malloc(53831) = 0
malloc(53830) = 0
malloc(53829) = 0
malloc(53828) = 0
malloc(53827) = 0
malloc(53826) = 0
malloc(53825) = 0
malloc(53824) = 0
malloc(53823) = 0
malloc(53822) = 0
malloc(53821) = 0
malloc(53820) = 0
malloc(53819) = 0
malloc(53818) = 0
malloc(53817) = 0
malloc(53816) = 0x3ffeedbc

[devyte]

You're doing a Serial.printf() in between the maxBlock and malloc operations, which could alloc. Then you have another printf in between malloc attempts, and yet throughout you keep assuming that the maxBlock hasn't changed.
Your test isn't quite correct.

[dplasa]

Thank you for clarifying my mistaken assumptions! My bad...
However, I tried to correct my test and it still fails (now by only 8 bytes...)

#include <ESP8266WiFi.h>

void setup() {
  Serial.begin(74880);
  WiFi.mode(WIFI_OFF);

  uint16_t maxBlock = ESP.getMaxFreeBlockSize();
  uint32_t freeHeap = ESP.getFreeHeap();

  Serial.print("free: ");
  Serial.print(freeHeap);
  Serial.print(" - max: ");
  Serial.println(maxBlock);

  // re-get maxBlock so no print is between
  // this and the first try to malloc()
  maxBlock = ESP.getMaxFreeBlockSize();

  void* p = NULL;
  while (p == NULL)
  {
    p = malloc(maxBlock);
    Serial.print("malloc(");
    Serial.print(maxBlock);
    Serial.print(") = ");
    Serial.println(p ? "success" : "fail");
    --maxBlock;
  }
}
void loop() {}```

And the debug output is

ets Jan 8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 1392, room 16
tail 0
chksum 0xd0
csum 0xd0
v3d128e5c
~ld
free: 54080 - max: 54072
malloc(54072) = fail
malloc(54071) = fail
malloc(54070) = fail
malloc(54069) = fail
malloc(54068) = success

[devyte]

Still not valid. Only the very first failed malloc is correct to fail (see explanation below). Then you print right after, at which point the maxBlock could change (would likely be smaller). You need to keep track of the state in variables, then print out the results only after you've reached a conclusion and your while loop has ended.

About malloc fail: the returned value of maxBlock is the "biggest chunk of contiguous free memory". It does not mean that you can allocate the entire block for yourself, because each allocation has an overhead to keep track.
We could potentially try to change the definition of that to mean "biggest allocatable chunk of contiguous free memory" to match what you're trying to do, but it would mean figuring out how to subtract the overhead from the currently returned value.
I'm not sure it's worth it, there are corner cases and special conditions to figure out which would likely be difficult to get right. In addition, the only way to assure that the maxBlock has zero chance of changing between the call that gets its value and the malloc that is immediately after is to have wifi off and no interrupts running. If either of those isn't true, all bets are off. The use case for that scenario is so limited as to be practically useless.
What is your intent here?

[dplasa]

OK, I'll do it all in variables:

#include <ESP8266WiFi.h>

uint8_t counter = 0;

#define MAXTRY 32

uint16_t maxBlockBefore[MAXTRY];
uint16_t maxBlockAfter[MAXTRY];
void* mallocResult[MAXTRY];

uint32_t freeHeap = ESP.getFreeHeap();

void setup() {
  Serial.begin(74880);
  WiFi.mode(WIFI_OFF);

  Serial.printf("free heap at start: %lu\n", freeHeap);
  delay(100);

  while (counter < MAXTRY)
  {
    noInterrupts();
    maxBlockBefore[counter] = ESP.getMaxFreeBlockSize(); // before 1st try
    mallocResult[counter] = malloc(maxBlockBefore[counter] - counter);
    maxBlockAfter[counter] = ESP.getMaxFreeBlockSize();  // after 1st try
    interrupts();
    if (mallocResult[counter])
      break;
    counter++;
  }

  for (auto i = 0; i <= counter; ++i)
  {
    Serial.printf("try %d: before: %u, malloc(%u): %s, after: %u\n", i, maxBlockBefore[i], maxBlockBefore[i] - i, mallocResult[i] ? "ok" : "fail", maxBlockAfter[i]);
  }
}

void loop()
{
  delay(100);
}

The output is

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 1392, room 16 
tail 0
chksum 0xd0
csum 0xd0
v3d128e5c
~ld
free heap at start: 54104
try 0: before: 53784, malloc(53784): fail, after: 53784
try 1: before: 53784, malloc(53783): fail, after: 53784
try 2: before: 53784, malloc(53782): fail, after: 53784
try 3: before: 53784, malloc(53781): fail, after: 53784
try 4: before: 53784, malloc(53780): ok, after: 8

Still, it only succeeds, if I malloc() 4 bytes less than ESP.getMaxFreeBlockSize() returns.

We could potentially try to change the definition of that to mean "biggest allocatable chunk of contiguous free memory" to match what you're trying to do, but it would mean figuring out how to subtract the overhead from the currently returned value.

To me, it looks like this overhead is 4 bytes ;-)

In addition, the only way to assure that the maxBlock has zero chance of changing between the call that gets its value and the malloc that is immediately after is to have wifi off and no interrupts running. If either of those isn't true, all bets are off. The use case for that scenario is so limited as to be practically useless.

I'm sure, that most people would agree, that ESP.getMaxFreeBlockSize() should return a value, that you can actually malloc(), even if special conditions would have to be met. But it's current behavior seems just plain wrong to me, since it never even works in principle, i.e. if the conditions are met.

What is your intent here?

I'm fiddeling with code for an FTP server and was thinking - naively - I could use that ESP.getMaxFreeBlockSize() call to determine the max buffer size for file transfers. I was also thinking of heap exhaustion and was only allocation that max block size if at least 10% of freeHeap stays free. However, with enough free heap, every so often, I just ended up, allocating just exactly the value returned by ESP.getMaxFreeBlockSize() which failed...

[devyte]

4 bytes is correct, but it is the overhead in your case. There are other cases, such as building with umm poison.
FTP means wifi enabled, so nope, trying to allocate the max block is not assured to work even if the overhead were taken into account. Wifi could receive something and allocate between the time the maxBlock was measured and the time the malloc was attempted, which would mean the measured maxBlock was no longer valid for alloc, and the alloc would still fail.
Reserving some % free of total heap also doesn't help as you might expect due to fragmentation. There are some approaches possible, like trying to keep a maxBlock above se threshold in reserve, but that also is a bit of a mess. A better approach would be to use a memory pool, which in your case would be the same a preallocating a big buffer, and then using/reusing it over and over.
My question to you is: why do you even want such a big buffer? In general, a buffer bigger than 1 or 2 x MSS doesn't make much sense, except for ssl where you need the encryption context, but that's a different story.
Outgoing data doesn't really need a buffer, and for incoming data I would stream to a scratch file, and delete it if something goes wrong.

[devyte]

BTW, #1183 proposes an ftp lib.

[earlephilhower]

While this is informative, I think the only bug here is "getMaxFreeBlockSize needs to subtract (epsilon)", no? Allocating the returned value should work (assuming no other allocs happen in between), but is an absolutely awful idea and will make your day a very bad one indeed.

[dplasa]

I'm might repeat myself, but I think ESP.getMaxFreeBlockSize() should return a value, that you can actually allocate if malloc() directly after it - every other behavior seems like a bug to me. Another way to look at the issue woul be to drop ESP.getMaxFreeBlockSize() altogether since it returns a value, that is almost of no use anyways ;-)

[dplasa]

BTW, #1183 proposes an ftp lib.

This project seems a little stale. I forked from that recently and worked on it.
I have

• LittleFS support (with directory handling)
• datestamp support
• also both active and passive mode support
• still work in progress* but it works alright with e.g. FileZilla
  Check it out, if you like: https://github.com/dplasa/esp8266FTPServer

*will not on esp32 although code claims to do so - i just ordered one to test it on that platform...

[earlephilhower]

I think your link got munged. The displayed URL is good, but the hyperlink doesn't seem to be. Neat work, though! https://github.com/dplasa/esp8266FTPServer

I'm might repeat myself, but I think ESP.getMaxFreeBlockSize() should return a value, that you can actually allocate if malloc() directly after it...

So, in this case, the returned value - 4 or (something else) when in UMM_DEBUG mode. I think we're in agreement on this point, if not on its importance.

It is definitely of use in determining how fragmented the system is, and what operations are no longer possible due to fragmentation (i.e. SSL requires 6K heap contiguous and 16.5K receive buffer, also configuous, and I2S can need 2K+ buffers, etc.)

However, allocating every single byte available, in an embedded system without MMU and which requires some amount of RAM randomly for binary blob-controlled WiFi connection ops or TCP transfers from the outside world, is going to bring you nothing but headaches when things fall over and die. Please, don't try it if you value your own time...

[dplasa]

Please, don't try it if you value your own time...

OK, I'll surrender! ;-)

I am planning to do some cleanups, (most likely make the transfer buffer static or just get rid of it at all), fix the readme.
Would be glad, if this were to be considered as part of the esp8266 libraries.