Memory leaks detected by valgrind

[JuergenKosel]

Describe the bug

As mentioned in #1487 (comment)
I had integrated the paho.mqtt.c of pull request #1487 to verify if this fixes a memory-leaks which was detected by valgrind in my application.

Unfortunally the memory leak still perists.

valgrind reported the following memory-leaks:

==17== error_begin
==17== 75 bytes in 1 blocks are possibly lost in loss record 46 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8FA3EA: mymalloc (Heap.c:177)
==17==    by 0x1B8D0A4F: Protocol_processPublication (MQTTAsyncUtils.c:2580)
==17==    by 0x1B8D8482: MQTTProtocol_handlePublishes (MQTTProtocolClient.c:339)
==17==    by 0x1B8D2D0B: MQTTAsync_cycle (MQTTAsyncUtils.c:3060)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end
==17== error_begin
==17== 13,380 (48 direct, 13,332 indirect) bytes in 1 blocks are definitely lost in loss record 143 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8E2601: TreeAddByIndex (Tree.c:248)
==17==    by 0x1B8E279B: TreeAdd (Tree.c:280)
==17==    by 0x1B8FA5F9: mymalloc (Heap.c:213)
==17==    by 0x1B8D0AC8: Protocol_processPublication (MQTTAsyncUtils.c:2586)
==17==    by 0x1B8D8482: MQTTProtocol_handlePublishes (MQTTProtocolClient.c:339)
==17==    by 0x1B8D2D0B: MQTTAsync_cycle (MQTTAsyncUtils.c:3060)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end
==17== error_begin
==17== 46,399 (48 direct, 46,351 indirect) bytes in 1 blocks are definitely lost in loss record 155 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8E2601: TreeAddByIndex (Tree.c:248)
==17==    by 0x1B8E279B: TreeAdd (Tree.c:280)
==17==    by 0x1B8FA5F9: mymalloc (Heap.c:213)
==17==    by 0x1B8DDE80: MQTTPacket_publish (MQTTPacket.c:563)
==17==    by 0x1B8DCB87: MQTTPacket_Factory (MQTTPacket.c:148)
==17==    by 0x1B8D2BFA: MQTTAsync_cycle (MQTTAsyncUtils.c:3047)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end
==17== error_begin
==17== 298,856 (192 direct, 298,664 indirect) bytes in 4 blocks are definitely lost in loss record 158 of 158
==17==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==17==    by 0x1B8E2601: TreeAddByIndex (Tree.c:248)
==17==    by 0x1B8E279B: TreeAdd (Tree.c:280)
==17==    by 0x1B8FA5F9: mymalloc (Heap.c:213)
==17==    by 0x1B8DD7A8: readUTFlen (MQTTPacket.c:393)
==17==    by 0x1B8DDEF9: MQTTPacket_publish (MQTTPacket.c:568)
==17==    by 0x1B8DCB87: MQTTPacket_Factory (MQTTPacket.c:148)
==17==    by 0x1B8D2BFA: MQTTAsync_cycle (MQTTAsyncUtils.c:3047)
==17==    by 0x1B8CE2DB: MQTTAsync_receiveThread (MQTTAsyncUtils.c:2032)
==17==    by 0x8CD6EA6: start_thread (pthread_create.c:477)
==17==    by 0x9458A2E: clone (clone.S:95)
==17== 
==17== error_end

To Reproduce

My application uses MQTT publish and subscribe.

Expected behavior
When the application terminates, all memory allocated by paho.mqtt.c is either released or the paho.mqtt.c libary holds still a pointer to that memory.

Environment:

• OS: Linux
• Version tag v1.3.10 , problem still exists in 43e2788 of Fix possible memory leak #1487

[icraggs]

Do you have a scenario or test program to produce this? Does it still exist, or is it any different in the latest release 1.3.13 or the develop branch?

[JuergenKosel]

Do you have a scenario or test program to produce this?

Unfortunately I have only my application and its tests, which I can not provide.

Does it still exist, or is it any different in the latest release 1.3.13 or the develop branch?

The issue still exists on the develop branch. (I reproduced with the single commit of #1487, which is based on the develop branch.
I think, that #1487 could already be on the right spot. But as commented there not sufficient.
At least I understood it this way, that in MQTTAsync_receiveThread() the allocated memory for pack should also be released.

Correct?

[icraggs]

Can you give me a sense of the activity and scale of your application? How many MQTT client objects, subscribe/publish activity, network interruptions, that sort of thing? How long does it run to produce this situation?

Perhaps a library trace at the protocol level?

The memory allocated in processPublication is normally freed in messageArrived by calling MQTTAsync_free and MQTTAsync_freeMessage normally. I've tried several tests of various kinds but not had any problem so far. This is with the latest develop branch.

[icraggs]

I'm sceptical that #1487 has any bearing on this. The only way that could be invoked would be a protocol error, and I will close that loophole. I don't think what you're seeing is a result of a protocol error on the part of the server.

[JuergenKosel]

Hello,

I just found out, that my colleague has not read the documentation properly:

/**
  * This function frees memory allocated to an MQTT message, including the
  * additional memory allocated to the message payload. The client application
  * calls this function when the message has been fully processed. <b>Important
  * note:</b> This function does not free the memory allocated to a message
  * topic string. It is the responsibility of the client application to free
  * this memory using the MQTTAsync_free() library function.
  * @param msg The address of a pointer to the ::MQTTAsync_message structure
  * to be freed.
  */
LIBMQTT_API void MQTTAsync_freeMessage(MQTTAsync_message** msg);

So the functions MQTTAsync_freeMessage() and MQTTAsync_free() are not called in his call back function at all.

[icraggs]

Hi Jürgen. A useful place to see example code is to look at the samples, paho_c_sub and paho_c_pub - they contain all the basic structure and usage.