bblanchon / ArduinoJson

📟 JSON library for Arduino and embedded C++. Simple and efficient.
https://arduinojson.org
MIT License
6.63k stars 1.1k forks source link

There is a memory leak either in this library or in ESP32 LittleFS #2103

Closed hitecSmartHome closed 1 week ago

hitecSmartHome commented 3 weeks ago

Environment
Here is the environment that I'm using':

Reproduction
Here is a small snippet that demonstrate the problem.

int read(const char* path, JsonDocument& doc){
    File file = userFS.open(path);
    if(!file){
        #if DB_DEBUG
            logger.write("[FileSystem] - (getJson) File not found: %s\n",path);
        #endif
        return -1;
    }
    DeserializationError error = deserializeJson(doc, file);
    file.close();
    if( error == DeserializationError::Ok ){
        #if DB_DEBUG
            logger.write("[FileSystem] - (getJson) Message read from file: %s\n",path);
        #endif
        return measureJson(doc);
    }
    return -1;
}

Program output
If relevant, include the program output.

Expected output:

No memory leak should happen

Actual output:

Memory leak.

(0) Free block 110592

[HwConfig] - Loaded 47612 bytes

(1) Free block 55296

(2) Free block 55296

My program never recovers from this and continues with free block of 55296 bytes. I'm measuring every min and it never recovers.

This is how I'm using the function


void printFreeBlockMem(int num){
    printf(
        "(%d) Free block %d\n",
        num,
        heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL)
    );
}

std::vector<Module> modules;

void load(){
  printFreeBlockMem(0); // Memory is OK

  JsonDocument doc;
  int size = read(HW_CONFIG_PATH, doc);
  if (size == -1) { return; )

  printf("[HwConfig] - Loaded %d bytes\n", size); // Read x bytes OK
  // Memory is NOT OK here but it's okay since the JsonDocument is still in scope
  // I would say that, but I have set malloc to allocate from External RAM and since
  // ArduinoJson 7 uses malloc() it should not be the case
  printFreeBlockMem(1);

  // Push modules into a vector
  for (JsonPair kv : doc.as<JsonObject>()) {
      JsonObject moduleObj = kv.value().as<JsonObject>();
      modules.push_back(Module(moduleObj));
  }
  modules.shrink_to_fit();
  printFreeBlockMem(2);
}

Since ArduinoJson 7 uses dynamic memory and I have setup malloc use external ram in my IDF config, I expect it to not cause any memory decrease from internal ram, but it does.

hitecSmartHome commented 3 weeks ago

The only thing I can think of is that the FS library somehow captures the pointer of the JsonDocument and does not release it. Because of that, the shared ptr of the JsonDocument never leaves the scope and I have no memory in result

bblanchon commented 3 weeks ago

Hi @hitecSmartHome,

I think you're confusing heap_caps_get_largest_free_block() with heap_caps_get_free_size():

size_t heap_caps_get_largest_free_block(uint32_t caps)

Get the largest free block of memory able to be allocated with the given capabilities. Returns the largest value of s for which heap_caps_malloc(s, caps) will succeed.

size_t heap_caps_get_free_size(uint32_t caps)

Get the total free size of all the regions that have the given capabilities. This function takes all regions capable of having the given capabilities allocated in them and adds up the free space they have.

Please try again with heap_caps_get_free_size().

Best regards, Benoit

hitecSmartHome commented 3 weeks ago

No I'm not confusing it. I know this is the largest free block at once that can be allocated and I need that. If I skip the deserialization from the file system my largest free block is enormus. Almost 3x the size.

The modules vector still filled with new Modules but not from the json file

hitecSmartHome commented 3 weeks ago

If I call the load() function but returning from it before it loads the modules, my system searches for new modules on the bus and still makes a valid modules vector with exactly the same information in them.

void load(){
  return; // Return so the loading never happens. Let the system add new modules instead of loading from the file system
  printFreeBlockMem(0); // Memory is OK

  JsonDocument doc;
  int size = read(HW_CONFIG_PATH, doc);
  if (size == -1) { return; )

  printf("[HwConfig] - Loaded %d bytes\n", size); // Read x bytes OK
  // Memory is NOT OK here but it's okay since the JsonDocument is still in scope
  // I would say that, but I have set malloc to allocate from External RAM and since
  // ArduinoJson 7 uses malloc() it should not be the case
  printFreeBlockMem(1);

  // Push modules into a vector
  for (JsonPair kv : doc.as<JsonObject>()) {
      JsonObject moduleObj = kv.value().as<JsonObject>();
      modules.push_back(Module(moduleObj));
  }
  modules.shrink_to_fit();
  printFreeBlockMem(2);
}
hitecSmartHome commented 3 weeks ago

If I do this without deserialization

printFreeBlockMem(0);
JsonDocument doc;
File file = db.userFS.open(HW_CONFIG_PATH, "r");
printFreeBlockMem(1);
return;

My ram is as before. Event without closing the file.

0 is before file open, 1 is after file open

(0) Free block 110592

(1) Free block 110592

If I close the file after I opened this is the same. If I put the deserialization in here

printFreeBlockMem(0);
JsonDocument doc;
File file = db.userFS.open(HW_CONFIG_PATH, "r");
DeserializationError error = deserializeJson(doc, file);
file.close();
printFreeBlockMem(1);
return;

This is the output

0) Free block 110592 

(1) Free block 49152

And it continues for the rest of the code. It never recovers. doc.clear() does not help.

hitecSmartHome commented 3 weeks ago

Will download the newest arduinojson and see if it solves the problem.

hitecSmartHome commented 3 weeks ago

I have downloaded the latest and put it in my sketch but the problem is still there.

hitecSmartHome commented 3 weeks ago

Interestingly it doesnt matter if I deserialize from a file or from a char buffer it always acts the same

int read(const char* path, JsonDocument& doc){
    File file = userFS.open(path);
    if(!file){ return -1; }

    printf("[TEST] - Path: %s\n", path);

    hshSystem.printFreeBlockMem(-1);
    // Allocate memory specifically from external ram!!
    char* buff = (char *)ps_malloc(file.size() + 1* sizeof(char));
    if( !buff ) ){ return -1; }
    // Print continous internal memory
    hshSystem.printFreeBlockMem(0);
    printf("[TEST] - Allocated buffer of size: %d\n", file.size() + 1);

    int read = file.readBytes(buff, file.size());
    // Print continous internal memory to determine if the problem is in the FS lib
    hshSystem.printFreeBlockMem(1);
    printf("[TEST] - Read %d bytes\n", read);

    DeserializationError error = deserializeJson(doc, buff, read);
    file.close();
    if( error ){ free(buff); return -1; }
    doc.shrinkToFit();
    hshSystem.printFreeBlockMem(2);
    printf("[TEST] - Deserialization successful\n");

    free(buff);
    return measureJson(doc);
}

Output:

[TEST] - Path: /hwConfig.json
(-1) Free block 110592
(0) Free block 110592
[TEST] - Allocated buffer of size: 54898
(1) Free block 110592    
[TEST] - Read 54897 bytes
(2) Free block 47104
[TEST] - Deserialization successful
hitecSmartHome commented 3 weeks ago

I have tried the Arduino_JSON library from the arduino-libraries repo.

https://github.com/arduino-libraries/Arduino_JSON/blob/master/examples/JSONValueExtractor/JSONValueExtractor.ino


// This function is from the offical lib examples of Arduino_JSON
void objRec(JSONVar myObject) {
    Serial.println("{");
    for (int x = 0; x < myObject.keys().length(); x++) {
        if ((JSON.typeof(myObject[myObject.keys()[x]])).equals("object")) {
            Serial.print(myObject.keys()[x]);
            Serial.println(" : ");
            objRec(myObject[myObject.keys()[x]]);
        } else {
            Serial.print(myObject.keys()[x]);
            Serial.print(" : ");
            Serial.println(myObject[myObject.keys()[x]]);
        }
    }
    Serial.println("}");
}

void loadModules(){
    // Check free block mem
    printFreeBlockMem(0);

    // Read the file content into String
    String buff;
    int size = db.read(HW_CONFIG_PATH, buff, db.userFS);
    // Print entire string
    printf("%s\n",buff.c_str());
    printf("[HwConfig] - Loaded %d bytes\n", size);

    // Check free block mem again
    printFreeBlockMem(1);

    JSONVar myObject = JSON.parse(buff);

    // Print json doc
    objRec(myObject);

    // Check free block mem again
    printFreeBlockMem(2);
}

My contignous free block of ram is dropping when parsing the object but it recovers successfully after that. I have also printed the file content to consoles which shows special characters but didn't find any suspicious char. No new line and nothing like that.

Output from Arduino_JSON

(0) Free block 110592

[HwConfig] - Loaded 54989 bytes

(1) Free block 102400

(2) Free block 43008

[HwConfig] - Scanning for modules...
[HwConfig] - Found 10 new modules.
(18941) Free block 102400

(20441) Free block 102400

(21941) Free block 102400

(23441) Free block 102400

As you can see my ram is dropped but just when the program was in the function.

hitecSmartHome commented 3 weeks ago

I have created an Arduino_JSON version of the required functions and methods in my sketch to be able to load my file correctly without using ArduinoJson

void HwConfig::loadModules(){
    hshSystem.printFreeBlockMem(0);

    String buff;
    int size = db.read(HW_CONFIG_PATH, buff, db.userFS);
    if( size == -1 ){ return; }

    JSONVar doc = JSON.parse(buff);
    JSONVar keys = doc.keys();

    for (int i = 0; i < keys.length(); i++) {
        JSONVar moduleObj = doc[keys[i]];
        modules.push_back( new Module(moduleObj) );
    }

    modules.shrink_to_fit();
    if (modules.size() > 0) {
        newModuleIndex = modules.size();
        hasSavedModules = true;
    }
    hshSystem.printFreeBlockMem(1);
}

I had to tweak my Module class a little bit too in order for this to work but the results speak for themselfs

(0) Free block 110592

(1) Free block 110592

(5315) Free block 98304

(6815) Free block 98304

(8315) Free block 98304

(9815) Free block 98304

(11315) Free block 98304

I have lost some ram but not that much with ArduinoJson

The same function with ArduinoJson

void HwConfig::loadModules() {
    hshSystem.printFreeBlockMem(0);

    JsonDocument doc;
    int size = db.read(HW_CONFIG_PATH, doc);
    if (size == -1) { return; }

    for (JsonPair kv : doc.as<JsonObject>()) {
        JsonObject moduleObj = kv.value().as<JsonObject>();
        modules.push_back( new Module(moduleObj) );
    }

    modules.shrink_to_fit();
    if (modules.size() > 0) {
        newModuleIndex = modules.size();
        hasSavedModules = true;
    }
    hshSystem.printFreeBlockMem(1);
}

(0) Free block 110592

(1) Free block 49152

(5315) Free block 49152

(6815) Free block 49152

(8315) Free block 49152

(9815) Free block 49152

(11315) Free block 49152
hitecSmartHome commented 3 weeks ago

The Arduino_JSON version is really crappy, it gives inconsistent results. Sometimes I have 90k free contignous free ram left, but sometimes it's just 67k but much better than the 30-40k with ArduinoJson

bblanchon commented 3 weeks ago

@hitecSmartHome, you claim that there is a memory leak in ArduinoJson, but you're measuring with the wrong tool. With heap_caps_get_largest_free_block(), all you can prove is that the heap is fragmented, which is a bummer but not a memory leak.

hitecSmartHome commented 3 weeks ago

Yeah, sorry for the spelling mistake. I did not stick in the details because of the fact that i lost almost 70k! contiguous memory in result of a deserialization. I dont say that the lib is faulty, i know it is a well tested and professionally written library. I just can't figure out the problem and everything points me to this lib. :/ I did not mean to be disrespectful.

bblanchon commented 2 weeks ago

This should not be a surprise since you repeatedly allocate blocks on the heap:

modules.push_back(new Module(moduleObj)

You could probably reduce the fragmentation by using a std::vector<Module> instead of std::vector<Module*>, and if called std::vector::reserve before deserializing.

hitecSmartHome commented 2 weeks ago

I rewrote all of this to store the pointers instead before, but it did not help.

hitecSmartHome commented 2 weeks ago

Originally I did it like this

std::vector<Module>
modules.push_back(Module(moduleObj);

I then started to store the pointers instead

std::vector<Module*>
modules.push_back(new Module(moduleObj);

No difference. Also the interesting thing is that the heap never recovers even if I do not fill the vector. I just deserialize the file.

hitecSmartHome commented 2 weeks ago
void printFreeBlockMem(int num){
    printf(
        "(%d) Free block %d\n",
        num,
        heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL)
    );
}

int read(const char* path, JsonDocument& doc){
    File file = userFS.open(path);
    if(!file){ return -1; }
    printFreeBlockMem(0);
    DeserializationError error = deserializeJson(doc, file);
    printFreeBlockMem(1);
    file.close();
    if( error ){ return -1; }
    return measureJson(doc);
}
(0) Free block 110592
(1) Free block 51200

I just can't narrow down the problem any more.

bblanchon commented 2 weeks ago

JsonDocument allocates memory on the heap, so it's perfectly normal to have less free heap after calling deserializeJson().

hitecSmartHome commented 2 weeks ago

Yeah but my heap stays the same after that. I wrote in one of my comments that it would be perfectly okay to stay that way as long as it is inside the scope of a function. With Arduino_JSON I don't have this behaviour despite the fact that, that lib is a mess.

I'm testing it with multiple modules. I have commented out almost all of my code around this call to make sure nothing affects the heap at the exact same time. I commented out the vector also, so no new objects are created, just the deserializing from file. I also wrote several functions which uses Arduino_JSON in place of my regular functions in this portion of my code base to lure out my code. The same function and same code base with Arduino_JSON does not produce this output. I have 100k free contignous heap and 180k free heap with Arduino_JSON. All of the other deserializing still uses ArduinoJson V7 hovewer. Rather strange. Maybe ArduinoJson V7 does not like this file. Maybe the file is too big and something is happening?

If I use ArduinoJson V7 I have 40k free contignous heap and 90k free heap after a deserialization for the rest of the runtime and not just in scope.

bblanchon commented 2 weeks ago

I have 100k free contignous heap and 180k free heap with Arduino_JSON. If I use ArduinoJson V7 I have 40k free contignous heap and 90k free heap

So, ArduinoJson consumes twice as much heap as Arduino_JSON in this situation, which is really bad. I expected to make huge memory optimizations in ArduinoJson 7.1, but my plans were disrupted by #2078. ArduinoJson 7.1 will instead be an all-MessagePack release, and memory optimizations are postponed to 7.2

hitecSmartHome commented 1 week ago

Thank you very much, will wait for 7.2 and retest.