From d4c72fd62333bf81c9dba4119be78d885f43e350 Mon Sep 17 00:00:00 2001
From: "Mark Landis (N6AZX)" <n6azx@fastmail.com>
Date: Sat, 17 Dec 2022 14:58:25 -0800
Subject: [PATCH] Default initialize class thread variables

During testing in a sandbox environment, xlxd crashed in
CG3Protocol::Close(). The root cause was dereferencing
m_pPresenceThread when the object had never been initialized.
In this case an error occured during ::Init() which set ok=false,
so the initialization of the threads was skipped.

This commit does 3 things:

(1) Default initializes the thread pointers to avoid the crash.
(2) Wraps the thread allocation with try/catch since std::thread can
    throw.
(3) Does some light cleaning in ::Close, e.g., converting NULL to
    nullptr.
---
 src/cg3protocol.cpp | 28 ++++++++++++++++++----------
 src/cg3protocol.h   |  8 +++++++-
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/cg3protocol.cpp b/src/cg3protocol.cpp
index 9f85420..770259f 100755
--- a/src/cg3protocol.cpp
+++ b/src/cg3protocol.cpp
@@ -79,9 +79,17 @@ bool CG3Protocol::Init(void)
     if (ok)
     {
         // start helper threads
-        m_pPresenceThread = new std::thread(PresenceThread, this);
-        m_pPresenceThread = new std::thread(ConfigThread, this);
-        m_pPresenceThread = new std::thread(IcmpThread, this);
+        try
+        {
+            m_pPresenceThread = new std::thread(PresenceThread, this);
+            m_pConfigThread = new std::thread(ConfigThread, this);
+            m_pIcmpThread = new std::thread(IcmpThread, this);
+        }
+        catch (const std::system_error& e)
+        {
+            ok = false;
+            // ... threads will be cleaned up on the call to ::Close()
+        }
     }
 #endif
     
@@ -94,25 +102,25 @@ bool CG3Protocol::Init(void)
 
 void CG3Protocol::Close(void)
 {
-    if (m_pPresenceThread != NULL)
+    if (m_pPresenceThread)
     {
         m_pPresenceThread->join();
         delete m_pPresenceThread;
-        m_pPresenceThread = NULL;
+        m_pPresenceThread = nullptr;
     }
 
-    if (m_pConfigThread != NULL)
+    if (m_pConfigThread)
     {
         m_pConfigThread->join();
         delete m_pConfigThread;
-        m_pConfigThread = NULL;
+        m_pConfigThread = nullptr;
     }
 
-    if (m_pIcmpThread != NULL)
+    if (m_pIcmpThread)
     {
         m_pIcmpThread->join();
         delete m_pIcmpThread;
-        m_pIcmpThread = NULL;
+        m_pIcmpThread = nullptr;
     }
 }
 
@@ -600,7 +608,7 @@ bool CG3Protocol::OnDvHeaderPacketIn(CDvHeaderPacket *Header, const CIp &Ip)
                         // drop if invalid module
                         delete Header;
                         g_Reflector.ReleaseClients();
-                        return NULL;
+                        return false;
                     }
                 }
 
diff --git a/src/cg3protocol.h b/src/cg3protocol.h
index 5ffdb79..5a05e96 100644
--- a/src/cg3protocol.h
+++ b/src/cg3protocol.h
@@ -64,7 +64,13 @@ class CG3Protocol : public CProtocol
 {
 public:
     // constructor
-    CG3Protocol() : m_GwAddress(0u), m_Modules("*"), m_LastModTime(0) {};
+    CG3Protocol() :
+        m_GwAddress(0u),
+        m_Modules("*"),
+        m_LastModTime(0),
+        m_pPresenceThread(nullptr),
+        m_pConfigThread(nullptr),
+        m_pIcmpThread(nullptr) {}
     
     // destructor
     virtual ~CG3Protocol() {};