eclipse-paho / paho.mqtt.cpp

Other
1.03k stars 439 forks source link

mqtt::async_client destructor should not close the session (send a disconnect request) #372

Closed nandlab closed 4 months ago

nandlab commented 2 years ago

Currently, the destructor of the mqtt::async_client class calls MQTTAsync_destroy, which in turn sends a disconnect request to close the mqtt session, then closes the TCP socket.

In paho.mqtt.cpp/src/async_client.cpp:

async_client::~async_client()
{
    MQTTAsync_destroy(&cli_);
}

In paho.mqtt.c/src/MQTTAsync.c:

void MQTTAsync_destroy(MQTTAsync* handle)
{
    MQTTAsyncs* m = *handle;

    FUNC_ENTRY;
    MQTTAsync_lock_mutex(mqttasync_mutex);

    if (m == NULL)
        goto exit;

    /********* Sending the disconnect request here *******/
    MQTTAsync_closeSession(m->c, MQTTREASONCODE_SUCCESS, NULL);

    MQTTAsync_freeResponses(m);
    MQTTAsync_freeCommands(m);
    ListFree(m->responses);

    [...]
}

This is not expected and can lead to problems. Instead, it should just disable the callbacks, close the TCP socket and deallocate all acquired resources. The destructor should not send any MQTT packets, because it is the responsibility of the programmer. If the programmer wants to close the session, he should call disconnect() explicitly.

My Use Case I want to track the status of a mqtt client. Therefore it has a status topic, where it publishes retained messages about its status. When connecting it sets will to publish FAILED on the status topic.

Example scenario: The client connected and set the status ONLINE. Now we imagine it runs out of memory. In the program, a fatal exception std::bad_alloc is thrown at some point, which causes the program to crash. While the program stack is unwinded, the mqtt::async_client destructor is called and the program exits abnormally. I would expect the status to become FAILED. Problem: Because the mqtt::async_client destructor closes the session, neither OFFLINE nor FAILED are published and the topic status stays at ONLINE, although the clients program crashed.

Can you please not close the session in the mqtt::async_client destructor?

icraggs commented 2 years ago

Actually what you're asking for is to not send the DISCONNECT. The session still needs to be closed (SOCKET closed, state cleaned up). And in MQTT V5 the DISCONNECT can be sent and request the LWT to be sent, so that could be used here.

fpagliughi commented 4 months ago

As the C++ library is attempting resource management via RAII, the client object does need to destruct the underlying C struct to avoid memory and resource leaks.

As to whether the C lib sends the DISCONNECT packet when the client struct it is being destroyed, that is not something that can be controlled externally, and should be more properly raised as an issue in the Paho C repository.

I suppose that, since the sending of a DISCONNECT packet is optional by the specification, then sending it could be an option in the C library. Perhaps that could be added as a boolean like int sendDisconnect in the MQTTAsync_disconnectOptions struct.

I'll open this issue in the C lib.