DanNixon / NeoNextion

Arduino library for the Nextion displays
https://dannixon.github.io/NeoNextion
GNU General Public License v2.0
48 stars 34 forks source link

Memory leak ! #37

Closed Bascy closed 1 year ago

Bascy commented 7 years ago

After discovering a memory leak in my app, I've narrowed it down to virtual inheritance of INextionWidget i.e. in INextionTouchable

INextionTouchable* p = nullptr; long mem = ESP.getFreeHeap(); p = new INextionTouchable(C->nextion, LIGHTSPAGE_PAGE_ID, 10, "TEST"); delete p; Serial.print(F("LEAK: ")); Serial.println(mem - ESP.getFreeHeap());

results in:

LEAK: 16

Is my conclusion correct, and if so, how can we solve this?

Bascy commented 7 years ago

Found the memory leak, its in Nextion::unregisterTouchable(INextionTouchable * touchable) which doesnt delete the ITouchableListItem it takes out of the linked list.

Further more, INextionTouchable adds itself to the list in the constructor, but it doesnt contain a destructor to unregister itsself from that list.

I dont have a original clone at the moment, so don't know if i can make a pull-request

DanNixon commented 7 years ago

The library is not designed to be used with anything other than compile time and initialisation time allocation, nor should any memory be freed.

This is a general principle applied when dealing with memory on embedded devices as you want to know as soon as possible if you are going to run out of memory (i.e. your program will either not compile or will fail to start).

Ideally you should be declaring your widgets in global scope (as per the examples) then their memory is statically allocated (with dynamic allocation happening at initialisation time). As widgets cannot be dynamically created on the display I see no good reason to create and destroy them on the application side.

Bascy commented 7 years ago

Ok if its not designed like this, but we're having over 15 pages in our nextion with >100 widgets, so i dynamically create the widgets as the pages become active... and destroy them again.