Memory Leak Issue

[masrodjie]

I was trying to loop write the date every seconds and it stopped for moment later (about 1 hour).

#include <Arduino.h>
#include <SPI.h>
#include <STM32SD.h>

char buff[128];
bool cardInitialized = true;

#define BUILTIN_LED1 PA6

bool writeFile(String writeString) {
  File dataFile = SD.open("TEST.txt", FILE_WRITE);
  if (dataFile) {
    dataFile.seek(dataFile.size());
    dataFile.println(writeString);
    dataFile.close();
    digitalWrite(BUILTIN_LED1, LOW);
    delay(50);
    digitalWrite(BUILTIN_LED1, HIGH);
    return true;
  } else{
    return false;
  }
}

void setup() {
  pinMode(BUILTIN_LED1, OUTPUT);
  pinMode(PA7, OUTPUT);
  pinMode(PE3, INPUT_PULLUP);
  pinMode(PE4, INPUT_PULLUP);
  digitalWrite(BUILTIN_LED1, HIGH);
  digitalWrite(PA7, HIGH);

  u_int8_t i = 0;
  while (!SD.begin()) {
    if (i > 10) {
      cardInitialized = false;
      break;
    }
    i++;
    delay(100);
  }
  
  writeFile("-----BEGIN-----");
}

void loop() {
  long randNumber;
  randNumber = random(100000);
  sprintf(buff,"%lu", randNumber);
  writeFile(buff);
  delay(1000);
}

Then i try to push the reset button, it start again then stop again in moment later.

Trying to debug using j-link and it stopped at:
https://github.com/stm32duino/STM32SD/blob/master/src/SD.cpp#L207
I think the problem is it unable to allocate memory so produce error handler and it stop.
I am using STM32F407VET6 https://github.com/mcauser/BLACK_F407VE

[fpistm]

Hi @masrodjie
In this case, you should better keep the file open as it is always the same moreover it will avoid to seek each time. You can use the flush to ensure the data are written.
Opening/closing will fragments the memory due to how newlib handle the allocation and this is why at a certain point it is not able to allocate anymore. Dynamic allocation should be used with parsimony in MCU env.

[masrodjie]

Hi @fpistm
Thank you for your quick response.
Ahhh sorry i was missing there are flush function. Previously, i was keep open but didn't put flush, so it never written. Just tested it whole night and it still running. If possible, please add example for loop write case so it may helpful for others. Or i can make PR if you like.

[fpistm]

Do not hesitate to make a PR. Any contribution are welcome 😉
Thanks in advance

[AnHardt]

Hmmm.
Looks like the underling problem could be:

The constructors of File use malloc() but there is no explicit destructor. So the allocated memory is never given back.

Because in the example of the first message dataFile is local in 'writeFile()' it is destroyed with every return. Memory is lost with every call of writeFile.

If that theory is correct, a global dataFile should work around the problem.

[fpistm]

Right, probably a destructor should call the close.

[masrodjie]

@AnHardt yes if you look into the Datalogger example, dataFile is global
https://github.com/stm32duino/STM32SD/blob/master/examples/Datalogger/Datalogger.ino

[AnHardt]

Investigated a bit more.
The leaking can be stopped by closing the failed to open file in the

  } else{
    return false;
  }

path. Closing a File what failed to open is not very intuitive.

---

So the losses only occurred when the file failed to open.
Opening the file failed on my Black_F407 randomly but quite often. Once failed, repeating to try to open did not help, it always failed then until reset. So in effect the memory loss was only a consequence of a brittle open().

EDIT:
The consecutive opening of the File fails with FR_TOO_MANY_OPEN_FILES. That seems to be a consequence of #define _FS_LOCK 2 in the ff-configuration. That could mean, internally the file is still open.

---

A dedicated destructor was not enough to fix the leak. Also a copy- and move-constructor is required. Instead I went an other way. See #31.