[mosquitto-dev] Thread race between mosquitto_disconnect and mosquitto_l

[Denis Dowling <dpd@xxxxxxxxxxxx>]

Hi All,

I am having some difficulty shutting down mosquitto client threads cleanly.

Attached is a small test program that does:

1) mosquitto_new()

2) mosquitto_connect()

3) mosquitto_loop_start()

4) mosquitto_disconnect()

5) sleep(1)

6) mosquitto_loop_stop()

If I run the program I get a MOSQ_ERR_INVAL return from 
mosquitto_loop_stop(). The mosq->threaded state is mosq_ts_none instead 
of the expected mosq_ts_self. This state gets changed when the 
mosquitto__thread_main() exists. This function changes the state from 
mosq_ts_self back to mosq_ts_none on thread exit.

I am stuck on how to deal with this in my code. If I ignore the error 
from mosquitto_loop_stop() then there is a small amount of memory leaked 
as the pthread_join() call is never called and this shows up on my 
valgrind reports.

Even without the sleep() call the program will sometimes fail if the 
mosquitto__thread_main() manages to exit before the 
mosquitto_loop_stop() gets called in the main thread. I have noticed 
this periodic failure on my unit tests.

I think the code in mosquitto__thread_main() that sets mosq->threaded 
back to mosq_ts_none should be removed or perhaps it needs an extra 
state for mosq_ts_self_exit.

Happy for someone to point out a better solution :-)

Regards,

Denis

#include <mosquitto.h>
#include <iostream>
#include <unistd.h>

int main()
{
    struct mosquitto *mosq;
    int ret;
    
    mosq = mosquitto_new(nullptr, true, nullptr);

    ret = mosquitto_connect(mosq, "localhost", 1883, 10);
    if (ret != MOSQ_ERR_SUCCESS)
    {
        std::cerr << "mosquitto_connect_async() failed\n";
        return 1;
    }

    ret = mosquitto_loop_start(mosq);
    if (ret != MOSQ_ERR_SUCCESS)
    {
        std::cerr << "mosquitto_loop_start() failed\n";
        return 1;
    }

    ret = mosquitto_disconnect(mosq);
    if (ret != MOSQ_ERR_SUCCESS)
    {
        std::cerr << "mosquitto_disconnect() failed\n";
        return 1;
    }

    // Wait for the disconnect to do something
    sleep(1);
    
    ret = mosquitto_loop_stop(mosq, false);
    if (ret != MOSQ_ERR_SUCCESS)
        std::cerr << "mosquitto_loop_stop() failed\n";

    mosquitto_destroy(mosq);
    
    return 0;
}