Fixing modules wthich don't use the Lua memory allocator

[TerryE]

The following modules include bare malloc / free API calls: coap, crypto, enduser_setup, file, http, mqtt, net, pcm, rotary, sntp, sqlite3, tls, tm1829, u8g_glue, websocket, wifi, wifi_eventmon, wifi_monitor, ws2812.

The lua allocator should always be used in Lua modules. The reason for this is that the Lua allocator is a thin wrapper on top of the standard allocator, but handles out of memory conditions differently. If this occurs, the Lua system runs the GC and then retries the allocation. It only fails is there there is not enough memory at this point. Also instead of returning a null pointer, a Lua error is thrown.

Expected behavior

Libraries should not raise a memory allocation error if the EGC mode ON_ALLOC_FAILURE is set and enough memory is available after GC to satisfy the allocation.

Actual behavior

The application can crash with an out of memory error even though there is enough heap available to satisfay the request.

NodeMCU version

master and dev

[pjsg]

I'm thinking that the easiest (although nasty) approach to this is to define a malloc/free wrapper over the lua allocator which allocates using the lua allocator, then stuffs the block into a table indexed by the memory address of the userdata. Then on free we can find the block in our table and remove that entry. I think that would preserve the correct semantics (apart from the out-of-memory condition when malloc would return NULL, and this approach would panic the system. This may not be bad as I bet that most places don't handle the out of memory condition gracefully)

[pjsg]

Of course, this still leaves the espressif core libraries which will continue to use their own malloc/free and maybe those do get it right.

[TerryE]

If you look at the code in mem.c the lua mem wrapper does end up doing an os_malloc without any framing record. It's just that if the malloc return NULL then it executes the GC and retries before dying. If the malloc are a one-off at library initialisation (which is the case in some of the libraries, then this isn't so much of an issue. (an this is mostly what the SDK routines do). It's when the modules do dynamic malloc/free calls that you can get the potential problems.

Oh, yes, whilst I think on the othe tweek that I would do would be to change the panic handler so that instead of just rebooting on stopping an uncaught error. it prints out the error traceback first so at least the developer can see where the uncaught error occurted.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[nwf]

Just some updates to note:

• sqlite3 is no more as of Remove sqlite3 module #3123
• tls's use of luaM_malloc() is going to go away eventually with espconn_secure/tls: eliminate funky flash game #3032, I hope.
• tm1829 and ws2812's allocations are going to go away with LED strip refactor #3158 (and the added pixbuf uses only userdata and stack allocations).

LwIP, unfortunately, uses raw malloc and friends internally (rather than having us pass it memory; a real anti-pattern in C libraries, but one so heavily entrenched as to require a time machine to undo), so tls is stuck with one call to free. The other LwIP-adjacent modules are in similar boats.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.