Potential memory leak about ebyte_lora_e22_series_library

[strud]

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]

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]

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]

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.

EByte_LoRa_E22_Series_Library/LoRa_E22.cpp

Line 780 in 510e04e

rc.data = malloc(size);

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]

Hi @sreekants,
you select the ResponseStructContainer that needs close

EByte_LoRa_E22_Series_Library/LoRa_E22.cpp

Lines 777 to 795 in 510e04e

	ResponseStructContainer LoRa_E22::receiveMessageComplete(const uint8_t size, bool rssiEnabled){
	ResponseStructContainer rc;
	rc.data = malloc(size);
	rc.status.code = this->receiveStruct((uint8_t *)rc.data, size);
	if (rc.status.code!=E22_SUCCESS) {
	return rc;
	}
	if (rssiEnabled){
	char rssi[1];
	this->serialDef.stream->readBytes(rssi, 1);
	rc.rssi = rssi[0];
	}
	this->cleanUARTBuffer();
	return rc;
	}

and It had It

EByte_LoRa_E22_Series_Library/LoRa_E22.h

Lines 228 to 235 in 510e04e

	struct ResponseStructContainer {
	void *data;
	byte rssi;
	ResponseStatus status;
	void close() {
	free(this->data);
	}
	};

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

EByte_LoRa_E22_Series_Library/LoRa_E22.h

Lines 228 to 240 in 510e04e

	struct ResponseStructContainer {
	void *data;
	byte rssi;
	ResponseStatus status;
	void close() {
	free(this->data);
	}
	};
	struct ResponseContainer {
	String data;
	byte rssi;
	ResponseStatus status;
	};

Bye Renzo

[sreekants]

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]

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