open_source / mqtt-cpp

When we delete our mqttclient instances (276 instances), sometimes we encounter a SEGFAULT originating in MqttClient::eventHandler. GDB always shows: 0x000055555576b4d5 in operator() (_closure=0x7ffea40023b0) at /source/mqtt-cpp/src/mqttclient.cpp:499. This is the line: principalClient->publishPending(); . principalClient pointer is not null, so the if-statement at 495 passes, but it is not a valid pointer. That is because the destructor already deleted that instance of principalClient (line 83, mprincipalClient.swap(principalClient); ). This is clearly indicated by this output:

(gdb) p m_eventQueue.m_stop

$1 = std::atomic = { true }

That indicates the destructor has already executed line 87.

This is caused by obtaining the classic pointer of m_principalClient at line 421, so principalClient is a reference which isn’t accounted for by the smart-pointer. I think there are 2 solutions to this:

Change the order in the destructor. Only delete m_principalClient after all events are completed, and m_workerThread.join() has completed.
Use smart-pointers for all references to m_principalClient, ensuring it will not be destroyed while events in the queue have a reference to it. 

This PR implements the first suggestion.

diff --git a/src/mqttclient.cpp b/src/mqttclient.cpp

index 2f7aef9..a36d6e6 100644

--- a/src/mqttclient.cpp

+++ b/src/mqttclient.cpp

@@ -73,6 +73,12 @@ MqttClient::MqttClient(const std::string& _clientId, const std::function

MqttClient::~MqttClient()

{

• LogDebug( "MqttClient", std::string( m_clientId + " - dtor stop queue" ) );

• m_eventQueue.stop();

• if (m_workerThread.joinable()) {

•    m_workerThread.join();
  

• }

+

 {

     // LogDebug( "MqttClient", std::string( m_clientId + " - disconnect" ) );

     this->disconnect();

@@ -82,12 +88,6 @@ MqttClient::~MqttClient()

     LogDebug( "MqttClient", std::string( m_clientId + " - cleanup principal client" ) );

     m_principalClient.swap(principalClient);

 }

-

• LogDebug( "MqttClient", std::string( m_clientId + " - dtor stop queue" ) );

• m_eventQueue.stop();

• if (m_workerThread.joinable()) {

•    m_workerThread.join();
  

• }

  LogDebug( "MqttClient", std::string( m_clientId + " - dtor ready" ) );

}