From 5b73edec8cbe916f2959e3c318b1e20dbb404614 Mon Sep 17 00:00:00 2001
From: Benjamin Sergeant <bsergeant@mz.com>
Date: Wed, 26 Jun 2019 19:12:48 -0700
Subject: [PATCH] make IXDNSLookup more robust

---
 ixwebsocket/IXDNSLookup.cpp | 23 +++++++++++------------
 ixwebsocket/IXDNSLookup.h   | 10 ++--------
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/ixwebsocket/IXDNSLookup.cpp b/ixwebsocket/IXDNSLookup.cpp
index bbb4991f..75898f55 100644
--- a/ixwebsocket/IXDNSLookup.cpp
+++ b/ixwebsocket/IXDNSLookup.cpp
@@ -9,10 +9,11 @@
 
 #include <string.h>
 #include <chrono>
+#include <thread>
 
 namespace ix
 {
-    const int64_t DNSLookup::kDefaultWait = 10; // ms
+    const int64_t DNSLookup::kDefaultWait = 1; // ms
 
     DNSLookup::DNSLookup(const std::string& hostname, int port, int64_t wait) :
         _hostname(hostname),
@@ -92,20 +93,19 @@ namespace ix
 
         int port = _port;
         std::string hostname(_hostname);
-        _thread = std::thread(&DNSLookup::run, this, self, hostname, port);
-        _thread.detach();
 
-        std::unique_lock<std::mutex> lock(_conditionVariableMutex);
+        // We make the background thread doing the work a shared pointer 
+        // instead of a member variable, because it can keep running when
+        // this object goes out of scope, in case of cancellation
+        auto t = std::make_shared<std::thread>(&DNSLookup::run, this, self, hostname, port);
+        t->detach();
 
         while (!_done)
         {
-            // Wait for 10 milliseconds on the condition variable, to see
-            // if the bg thread has terminated.
-            if (_condition.wait_for(lock, std::chrono::milliseconds(_wait)) == std::cv_status::no_timeout)
-            {
-                // Background thread has terminated, so we can break of this loop
-                break;
-            }
+            // Wait for 1 milliseconds, to see if the bg thread has terminated.
+            // We do not use a condition variable to wait, as destroying this one
+            // if the bg thread is alive can cause undefined behavior.
+            std::this_thread::sleep_for(std::chrono::milliseconds(_wait));
 
             // Were we cancelled ?
             if (isCancellationRequested && isCancellationRequested())
@@ -140,7 +140,6 @@ namespace ix
             setRes(res);
             setErrMsg(errMsg);
 
-            _condition.notify_one();
             _done = true;
         }
     }
diff --git a/ixwebsocket/IXDNSLookup.h b/ixwebsocket/IXDNSLookup.h
index 60becaca..e56c2816 100644
--- a/ixwebsocket/IXDNSLookup.h
+++ b/ixwebsocket/IXDNSLookup.h
@@ -12,10 +12,9 @@
 
 #include "IXCancellationRequest.h"
 #include <atomic>
-#include <condition_variable>
+#include <mutex>
 #include <set>
 #include <string>
-#include <thread>
 #include <memory>
 
 struct addrinfo;
@@ -52,8 +51,8 @@ namespace ix
 
         std::string _hostname;
         int _port;
-
         int64_t _wait;
+        const static int64_t kDefaultWait;
 
         struct addrinfo* _res;
         std::mutex _resMutex;
@@ -62,10 +61,5 @@ namespace ix
         std::mutex _errMsgMutex;
 
         std::atomic<bool> _done;
-        std::thread _thread;
-        std::condition_variable _condition;
-        std::mutex _conditionVariableMutex;
-
-        const static int64_t kDefaultWait;
     };
 } // namespace ix