From 0bdee6ec99f5ac34d4ef38a1aa22c9a11a8dca54 Mon Sep 17 00:00:00 2001
From: Thomas Basler <thomas@familie-basler.net>
Date: Thu, 3 Aug 2023 18:46:28 +0200
Subject: [PATCH] Fix: Prevent access to nullptr object when reconnecting to
 mqtt

It could occour when saving the settings via the web ui
---
 include/MqttSettings.h |  2 ++
 src/MqttSettings.cpp   | 45 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/include/MqttSettings.h b/include/MqttSettings.h
index e68c230f..68b12c9c 100644
--- a/include/MqttSettings.h
+++ b/include/MqttSettings.h
@@ -5,6 +5,7 @@
 #include <MqttSubscribeParser.h>
 #include <Ticker.h>
 #include <espMqttClient.h>
+#include <mutex>
 
 class MqttSettingsClass {
 public:
@@ -37,6 +38,7 @@ private:
     String willTopic;
     Ticker mqttReconnectTimer;
     MqttSubscribeParser _mqttSubscribeParser;
+    std::mutex _clientLock;
 };
 
 extern MqttSettingsClass MqttSettings;
\ No newline at end of file
diff --git a/src/MqttSettings.cpp b/src/MqttSettings.cpp
index a8829166..81624455 100644
--- a/src/MqttSettings.cpp
+++ b/src/MqttSettings.cpp
@@ -32,21 +32,30 @@ void MqttSettingsClass::onMqttConnect(bool sessionPresent)
     const CONFIG_T& config = Configuration.get();
     publish(config.Mqtt_LwtTopic, config.Mqtt_LwtValue_Online);
 
-    for (const auto& cb : _mqttSubscribeParser.get_callbacks()) {
-        mqttClient->subscribe(cb.topic.c_str(), cb.qos);
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient != nullptr) {
+        for (const auto& cb : _mqttSubscribeParser.get_callbacks()) {
+            mqttClient->subscribe(cb.topic.c_str(), cb.qos);
+        }
     }
 }
 
 void MqttSettingsClass::subscribe(const String& topic, uint8_t qos, const espMqttClientTypes::OnMessageCallback& cb)
 {
     _mqttSubscribeParser.register_callback(topic.c_str(), qos, cb);
-    mqttClient->subscribe(topic.c_str(), qos);
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient != nullptr) {
+        mqttClient->subscribe(topic.c_str(), qos);
+    }
 }
 
 void MqttSettingsClass::unsubscribe(const String& topic)
 {
     _mqttSubscribeParser.unregister_callback(topic.c_str());
-    mqttClient->unsubscribe(topic.c_str());
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient != nullptr) {
+        mqttClient->unsubscribe(topic.c_str());
+    }
 }
 
 void MqttSettingsClass::onMqttDisconnect(espMqttClientTypes::DisconnectReason reason)
@@ -97,6 +106,12 @@ void MqttSettingsClass::performConnect()
         using std::placeholders::_4;
         using std::placeholders::_5;
         using std::placeholders::_6;
+
+        std::lock_guard<std::mutex> lock(_clientLock);
+        if (mqttClient == nullptr) {
+            return;
+        }
+
         MessageOutput.println("Connecting to MQTT...");
         const CONFIG_T& config = Configuration.get();
         willTopic = getPrefix() + config.Mqtt_LwtTopic;
@@ -132,6 +147,10 @@ void MqttSettingsClass::performDisconnect()
 {
     const CONFIG_T& config = Configuration.get();
     publish(config.Mqtt_LwtTopic, config.Mqtt_LwtValue_Offline);
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient == nullptr) {
+        return;
+    }
     mqttClient->disconnect();
 }
 
@@ -147,6 +166,10 @@ void MqttSettingsClass::performReconnect()
 
 bool MqttSettingsClass::getConnected()
 {
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient == nullptr) {
+        return false;
+    }
     return mqttClient->connected();
 }
 
@@ -157,6 +180,11 @@ String MqttSettingsClass::getPrefix()
 
 void MqttSettingsClass::publish(const String& subtopic, const String& payload)
 {
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient == nullptr) {
+        return;
+    }
+
     String topic = getPrefix();
     topic += subtopic;
 
@@ -168,6 +196,10 @@ void MqttSettingsClass::publish(const String& subtopic, const String& payload)
 
 void MqttSettingsClass::publishGeneric(const String& topic, const String& payload, bool retain, uint8_t qos)
 {
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient == nullptr) {
+        return;
+    }
     mqttClient->publish(topic.c_str(), qos, retain, payload.c_str());
 }
 
@@ -181,8 +213,11 @@ void MqttSettingsClass::init()
 
 void MqttSettingsClass::createMqttClientObject()
 {
-    if (mqttClient != nullptr)
+    std::lock_guard<std::mutex> lock(_clientLock);
+    if (mqttClient != nullptr) {
         delete mqttClient;
+        mqttClient = nullptr;
+    }
     const CONFIG_T& config = Configuration.get();
     if (config.Mqtt_Tls) {
         mqttClient = static_cast<MqttClient*>(new espMqttClientSecure);