xreef / EByte_LoRa_E22_Series_Library

Arduino LoRa EBYTE E22 device library complete and tested with Arduino, esp8266, esp32, STM32 and Raspberry Pi Pico (rp2040 boards).. sx1262/sx1268
Other
104 stars 22 forks source link

Potential memory leak #14

Open sreekants opened 1 year ago

sreekants commented 1 year ago

In LoRa_E22.cpp a buffer is allocated in each call

... ResponseStructContainer LoRa_E22::receiveMessageComplete(const uint8_t size, bool rssiEnabled){ ResponseStructContainer rc;

rc.data = malloc(size);
   ...

However, in all of the samples, you appear not to release the memory, for example esp32_e22_04_SendFixedTransmission.ino:

... if (e22ttl.available()>1) { // read the String message

ifdef ENABLE_RSSI

ResponseContainer rc = e22ttl.receiveMessageRSSI();

else

ResponseContainer rc = e22ttl.receiveMessage();

endif

...

Isn't rc.close() required to release the memory? Also, is there a way to avoid malloc() every time you receive a message to avoid fragmentation?

strud commented 1 year ago

Hi sreekants,

I had this issue recently and yes you must call rc.close() every cycle to release the memory.

I have also suggested it be added to the example code.

Maybe a better approach is to allocate the maximum allowable per message ie 256 bytes and then leave it allocated for ever?

sreekants commented 1 year ago

Hi sreekants,

I had this issue recently and yes you must call rc.close() every cycle to release the memory.

I have also suggested it be added to the example code.

Maybe a better approach is to allocate the maximum allowable per message ie 256 bytes and then leave it allocated for ever?

I like the idea of a pre-allocated buffer managed by the library. That would be a good fix with backward compatibility if the close() function has an empty body.

xreef commented 1 year ago

Hi all, no, the ResponseContainer doesn't need a close function because there isn't a pointer but directly a complex variable. Bye Renzo

sreekants commented 1 year ago

Hi Renzo, Where is the corresponding free() to the buffer allocated on line 780? The pointer rc.data holds a pointer to this buffer, but the only call releasing this buffer is ResponseContainer::close(). However, I do not see it being called.

https://github.com/xreef/EByte_LoRa_E22_Series_Library/blob/510e04e617948b65bda5bda8dd50986fa4cbd231/LoRa_E22.cpp#L780

Can you explain how the heap is managed correctly with the library?

(We arrived at this problem because we were transmitting 220 byte frame over LoRa E22 in our software and when we monitored the heap it fell from 180Kb, steadily to 70kb as our stress tests ran through a few hours.)

xreef commented 1 year ago

Hi @sreekants, you select the ResponseStructContainer that needs close

https://github.com/xreef/EByte_LoRa_E22_Series_Library/blob/510e04e617948b65bda5bda8dd50986fa4cbd231/LoRa_E22.cpp#L777-L795

and It had It

https://github.com/xreef/EByte_LoRa_E22_Series_Library/blob/510e04e617948b65bda5bda8dd50986fa4cbd231/LoRa_E22.h#L228-L235

but the ResponseContainer doesn't have and doesn't need a close.

https://github.com/xreef/EByte_LoRa_E22_Series_Library/blob/510e04e617948b65bda5bda8dd50986fa4cbd231/LoRa_E22.h#L228-L240

Bye Renzo

sreekants commented 1 year ago

Yes, I understand now. I stand corrected. Your samples use ResponseContainer. So they do not need a close(). You are correct there.

In our usecase, however, we are sending binary data over LoRa. The ResponseContainer is suitable only for string data. This is why we end up calling receiveMesssageComplete(). As stated, each call allocates a buffer, fragmenting memory, even if the close() function is called. Is there a better way to allow the use of a preallocated buffer so we don't have to malloc() every loop?

xreef commented 1 year ago

Ahhh ok, now I understand, mmm... no I don't think about that, but It's an interesting feature and can be a good enhancement. I think of a solution. If you have some suggestions write here. Bye Renzo

fixajteknik commented 1 month ago

I think I asked that question and you made some update and then I share a video about this issue 2 years ago https://github.com/xreef/EByte_LoRa_E22_Series_Library/issues/8