possible race condition between MQTTClient_destroy and MQTTClient_run

[wangkevin5626]

Describe the bug
When app's main thread calls MQTTClient_destroy，we still have backgroud thread running which is "MQTTClient_run", due to the production range of "mqttclient_mutex" is not sufficient, the "bstate->clients" lists maybe deleted by MQTTClient_destroy, so the background thread will trigger coredump (use after free). CallStack is as blow (paho.mqtt version is v1.3.12):

Thread 1 (Thread 0xe7fb65fbeca0 (LWP 2376)):
#0 MQTTProtocol_retry (now=..., doRetry=0, regardless=0) at /home/paho/src/MQTTProtocolClient.c:910
#1 0x0000e7fbe3840714 in MQTTClient_retry () at /home/paho/src/MQTTClient.c:2561
#2 MQTTClient_cycle (sock=, sock@entry=0xe7fb65fbe350, timeout=, timeout@entry=100, rc=, rc@entry=0xe7fb65fbe354) at /home/paho/src/MQTTClient.c:2662
#3 0x0000e7fbe3841774 in MQTTClient_run (n=) at /home//paho/src/MQTTClient.c:843
#4 0x0000e7fbe1163f20 in start_thread (arg=0xe7fb78f1dd47) at pthread_create.c:443
#5 0x0000e7fbe11cca9c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone.S:79
(gdb)

[icraggs]

You need to wait for any background thread to end before calling destroy. That's normally done by calling disconnect with a long enough timeout.

[wangkevin5626]

App cannot access backgound thread(e.g. mqtt run thread)，so if app calls MQTTClient_Destroy, MQTTClient_Destroy function need to wait run thread end

[e493956170]

You need to wait for any background thread to end before calling destroy. That's normally done by calling disconnect with a long enough timeout.

Is there a better solution besides waiting? The behavior can't not ensure working good in the severe situation.

[Zephyrus29]

I'm seeing a similar bug. I'm calling disconnect with a 10 minute timeout, then destroy.
Sometimes, not all the time, it crashes in the client thread.

I think there's a bug where when MQTTClient_terminate is called, it then calls MQTTClient_stop. If there's an active connection, it won't stop the thread (conn_count is non-zero).
However, it then returns and frees and destroys all the objects even though the thread is still running.