Memory leak -> Crash (WiFiClient connection)

I seem to have a memory leak which causes the processor to crash.
Using ESP8266 (NodeMCU) board.

I've cut my code down to bare minimum to demonstrate the issue.
Application prints out loop count and free heap size.
Starts with: "LoopCount: 1, FreeHeap: 47152"
Ends with: "LoopCount: 255, FreeHeap: 416"

After that there are some crash report messages:

Exception (29):
epc1=0x4000e1b2 epc2=0x00000000 epc3=0x00000000 excvaddr=0x00000000 depc=0x00000000

ctx: sys 
sp: 3ffffd70 end: 3fffffb0 offset: 01a0

Then the processor hangs and eventually resets.

#include <ESP8266WiFi.h>

// WiFi settings
const char* ssid     = "SSID";
const char* password = "PASSWORD";

// CherryPy Server Access
const char* myServer = "192.168.2.4";

void setup() {
 Serial.begin(250000);
 WiFi.disconnect();
 WiFi.mode(WIFI_STA);
 WiFi.begin(ssid, password);
 while (WiFi.status() != WL_CONNECTED) {
 delay(500);
 Serial.print(".");
 }
}

int loopCount = 0;

void loop() {
 String POSTrLine;
 WiFiClient client;
 loopCount++;
 digitalWrite(LED_BUILTIN, !digitalRead(LED_BUILTIN)); // Toggle RED led

 if(!client.connected()){
 if(client.connect(myServer, 80)){
 Serial.printf("New Connection to: %s\n", myServer);
 }
 }
 else {
 Serial.printf("Existing Connection to: %s\n", myServer);
 }

 while(client.available()){
 POSTrLine = client.readStringUntil('\r');
 }
 client.stop();
 Serial.printf("LoopCount: %u, FreeHeap: %u\n", loopCount, ESP.getFreeHeap()); 
}

EDIT: ISSUE RESOLVED
see Memory leak ESP8266WebServer · Issue #230 · esp8266/Arduino · GitHub

I'm not familiar with the library, but I don't see anything immediately jumping out of your code that looks like it could cause the memory leak. The fault may well be in the library itself. You need to experiment a bit more to narrow down the trigger, eg.

  • make WiFiClient client; a global, see if the behaviour changes
  • remove the client.stop() and let client.connect() implicitly disconnnect
  • try without the readStringUntil()
  • ...

And then have a look in the library source code for possible candidate leaks.

Maybe someone else has more ides.

	String POSTrLine;

QUIT USING Strings.

PaulS:

	String POSTrLine;

QUIT USING Strings.

I know String is frowned upon in the forum here, but if it is so bad that it causes problems even in this small piece of code, then it is basically useless.

Anyway, if that could be the cause of the problem, then the OP will need to swap readStringUntil() for size_t readBytesUntil(char terminator, char *buffer, size_t length)

Your usage of

 POSTrLine = client.readStringUntil('\r');

is incorrect YOU are required to delete and do mean use delete the String returned by .readStringUntil().

Of course use of blocking functions is stupid!

Mark

I don't think usage of String and client.readstringuntil('\r') is the issue here.
I've tried changing the line to:

char POSTrLine[600];
client.readBytesUntil('\r', POSTrLine, sizeof(POSTrLine));

but what I found out is that the application never even gets to:

while(client.available()){
		client.readBytesUntil('\r', POSTrLine, sizeof(POSTrLine));
                Serial.println("CLIENT AVAILABLE");
	}

Which, in my case may be fine because I'm not actually sending anything to the server, so will not be getting anything back. Just connecting / disconnecting for this debug code.

holmes4:
Your usage of

 POSTrLine = client.readStringUntil('\r');

is incorrect YOU are required to delete and do mean use delete the String returned by .readStringUntil().

Of course use of blocking functions is stupid!

Mark

readStringUntil() returns a String object on the stack, and it also appears to offer some sort of time-out mechanism (probably 5 seconds in this case):

String Stream::readStringUntil(char terminator) {
    String ret;
    int c = timedRead();
    while(c >= 0 && c != terminator) {
        ret += (char) c;
        c = timedRead();
    }
    return ret;
}

readStringUntil() returns a String object on the stack,

IT CANNOT RETURN ANYTHING ON THE STACK! Look up how the stack works!

Mark

holmes4:
IT CANNOT RETURN ANYTHING ON THE STACK! Look up how the stack works!

Mark

OK. A search for "how a stack works" returned this page as the first hit.

As you can see, by placing the arguments on the stack, the stack frame for main() has increased in size. We also reserved some space for the return value. The return value is computed by foo(), so it will be filled out once foo() is done.

holmes4:
Your usage of

 POSTrLine = client.readStringUntil('\r');

is incorrect YOU are required to delete and do mean use delete the String returned by .readStringUntil().

Your advice is incorrect. There are hidden ways that this local variable is assigned a return value from the call. You should not delete POSTrLine, because it is a local variable (on the stack), not something created with new (from the heap).

holmes4:
IT CANNOT RETURN ANYTHING ON THE STACK! Look up how the stack works!

Objects can be returned from functions. They are on the stack. I think you need to look up how C++ manages this for you. Try this and this.

holmes4:
Of course use of blocking functions is stupid!

+1

arduarn:
then [String] is basically useless.

+1

The OP's problem is probably caused by the combination of:

  • Unbounded heap usage by String (i.e., no limits on String length)

  • Limited RAM due to large libraries (increases likelihood of stack and heap colliding)

  • Rapidly constructing/destructing and connecting/disconnecting the WiFiClient (thrashing network activities)

I've eliminated the use of String.
The WiFiClient client variable does get created on every loop / iteration but it should be cleared / destroyed.

Rapidly constructing/destructing and connecting/disconnecting the WiFiClient (thrashing network activities)

I don't see the difference between running the loop every second vs every hour.

I don't see the difference between running the loop every second vs every hour.

Since I can't see the code you are talking about, do you mean that if you wrap your loop with a timer like this:

void loop() {

 if (millis() - lastTime >= (60 * 60 * 1000UL)) {
 lastTime = millis();

 String POSTrLine;
 WiFiClient client;
 loopCount++;
 digitalWrite(LED_BUILTIN, !digitalRead(LED_BUILTIN)); // Toggle RED led
 
 if(!client.connected()){
 if(client.connect(myServer, 80)){
 Serial.printf("New Connection to: %s\n", myServer);
 }
 }
 else {
 Serial.printf("Existing Connection to: %s\n", myServer);
 }

 while(client.available()){
 POSTrLine = client.readStringUntil('\r');
 }
 client.stop();
 Serial.printf("LoopCount: %u, FreeHeap: %u\n", loopCount, ESP.getFreeHeap());
 }
}

... it crashes while waiting?

I don't like to guess what you tried, so please post the code for each test you tried.

Did you try arduarn's suggestions in reply #1?

I was confused by your comment:

  • Rapidly constructing/destructing and connecting/disconnecting the WiFiClient (thrashing network activities)

Adding a delay(100), or delay(5000) at the end of the loop{} doesn't change the result. FreeHeap still keeps going down with every iteration and eventually gets close to zero -> crashes.

Definitely sounds like a memory leak.

DID YOU TRY ARDUARN'S SUGGESTIONS IN REPLY #1?

The first suggestion would pin the problem on constructing/destructing the WiFiClient class.

The second suggestion would pin the problem on ::stop.

Either one would be a work-around. It would also help limit the search boundary... if you want to try to fix the WiFi code.

  • Making WiFiClient client global by itself does not solve the issue.
  • Removing client.stop() by itself does not solve the issue.
  • Making the WiFiClient client global AND removing client.stop() seems to help.

The HeapSize is stable but what happens is that the connection never closes. Not sure if that is acceptable from the server side...

What I've gathered is that client.connect() generates a new instance every time... Not a C++ expert so need to learn / understand the underlying libraries.

Where exactly did you get the library from? The exact version/revision of the code could be important.

arduarn:
Where exactly did you get the library from? The exact version/revision of the code could be important.

The library is installed through the board manager after selecting "esp8266 by ESP8266 Commnunity version 2.3.0"

I did find the solution to the issue:
It is a known issue addressed here: Memory leak ESP8266WebServer · Issue #230 · esp8266/Arduino · GitHub
TCP connection does close and free up memory but it takes some time. So frequent connection eat-up all the memory.

Possible solution from the link above:

ClientContext.h

64 err = tcp_close(_pcb);
65 tcp_abort(_pcb); // Modification 28-06-2015
66 if(err != ERR_OK) {
67 DEBUGV(":tc err %d\r\n", err);
68 tcp_abort(_pcb);
69 err = ERR_ABRT;

Glad you found a solution and thanks for letting us know.