LwIP Socket Memory Leak

[caseymdk]

Hello!

So, I've been doing some work on TCP sockets with the ESP32, and it appears I have run into a memory leak that only appears if the socket failed to open.

I have attached code for a sample program that reproduces the bug.

In menuconfig, set the correct WiFi credentials for your network under "Example Configuration" -> "WiFi SSID"/"WiFi Password"

Then set "Component Config" -> "LWIP" -> "Max number of open sockets" to 1. This will make the code fail to open a socket for incoming connections.

Then, run (under linux bash) the command "watch -n 0.25 telnet <ESP32_IP> 5000". This will attempt to connect to the open socket multiple times, but will fail each time because it's trying to open more than the max number of supported open sockets.

Despite the fact that the socket does not open, you can watch the free heap slowly decrease down to 0. The memory for the socket should be freed, but it is not.

This appears to be related to bug #784 here: https://github.com/espressif/esp-idf/issues/784

The same occurs for "socket()" rather than "accept()" calls, if there are too many sockets open on the system.

Tested under tag release/v3.0, as well as master (4b91c82c)

If I add "netconn_free(newconn);" under line 739 of "esp-idf/components/lwip/api/sockets.c", the leak goes away.

This should be fixed in other locations too, for example, adding "netconn_free(conn);" under line 1506 of "lwip_socket()" in the same file as "lwip_accept()".

I know that this is changing the LWIP code, rather than the ESP-IDF code, but I suspect there is code somewhere in the ESP-IDF that would do the same thing as what I'm doing by adding "netconn_free()". It looks like the fix for bug 784 was done in this manner.

Let me know if I'm on the right track here.

Thanks!

[ESP_Angus]

Hi Casey,

Thanks for the detailed bug report. I was able to reproduce and have committed a fix for review internally.

(You were on the right track: there is a static function is_created_by_socket() in api_lib.c which checks if a netconn has a controlling socket. If it does, netconn_free() is deferred until later, to support multi-thread socket operations. However LWIP sometimes calls conn->socket-- to register activity on a netconn with no controlling socket, therefore the check needs to be >= 0 rather than != 1).

Angus

[caseymdk]

Fantastic, thanks Angus! Happy to help.

[caseymdk]

Hi Casey,

Thanks for the detailed bug report. I was able to reproduce and have committed a fix for review internally.

(You were on the right track: there is a static function is_created_by_socket() in api_lib.c which checks if a netconn has a controlling socket. If it does, netconn_free() is deferred until later, to support multi-thread socket operations. However LWIP sometimes calls conn->socket-- to register activity on a netconn with no controlling socket, therefore the check needs to be >= 0 rather than != 1).

Angus

Hey Angus,

Do you know if the fix passed internal review? I don't see it in the master branch yet. I am curious to look at the changes and see how they work. No rush, just curious, thanks!

[ESP_Angus]

Hi casey,

Sorry I was away for a few days last week. Fix was merged this morning:
https://github.com/espressif/esp-idf/co ... 188172283c

Please let us know if you still experience any issues.


Angus