From 3b11b80b10c95a87f303c59617cbaccf4ec9022f Mon Sep 17 00:00:00 2001
From: Nitromelon <hwc14@qq.com>
Date: Wed, 18 May 2022 13:13:59 +0800
Subject: [PATCH] Fix core dump causing by logging in destructor. (#1250)

Co-authored-by: Martin Chang <marty1885@users.noreply.github.com>
Co-authored-by: an-tao <antao2002@gmail.com>
---
 lib/src/HttpAppFrameworkImpl.cc | 25 +++++++++++++++++++------
 lib/src/HttpAppFrameworkImpl.h  | 20 ++++++++++----------
 lib/src/ListenerManager.cc      | 32 ++++++++++++++++++--------------
 lib/src/ListenerManager.h       |  1 +
 lib/src/StaticFileRouter.cc     | 16 +++++++++-------
 lib/src/StaticFileRouter.h      |  2 +-
 6 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/lib/src/HttpAppFrameworkImpl.cc b/lib/src/HttpAppFrameworkImpl.cc
index 81eb5996..4f187de0 100644
--- a/lib/src/HttpAppFrameworkImpl.cc
+++ b/lib/src/HttpAppFrameworkImpl.cc
@@ -1012,7 +1012,19 @@ void HttpAppFrameworkImpl::quit()
     if (getLoop()->isRunning())
     {
         getLoop()->queueInLoop([this]() {
+            // Release members in the reverse order of initialization
             listenerManagerPtr_->stopListening();
+            websockCtrlsRouterPtr_.reset();
+            staticFileRouterPtr_.reset();
+            httpSimpleCtrlsRouterPtr_.reset();
+            httpCtrlsRouterPtr_.reset();
+            pluginsManagerPtr_.reset();
+            redisClientManagerPtr_.reset();
+            dbClientManagerPtr_.reset();
+            // TODO: let HttpAppFrameworkImpl manage IO loops
+            // and reset listenerManagerPtr_ before IO loops quit.
+            listenerManagerPtr_->stopIoLoops();
+            listenerManagerPtr_.reset();
             getLoop()->quit();
         });
     }
@@ -1119,15 +1131,16 @@ HttpAppFramework &HttpAppFrameworkImpl::setupFileLogger()
             {
                 baseName = "drogon";
             }
-            asyncFileLoggerPtr_ = std::make_unique<trantor::AsyncFileLogger>();
+            asyncFileLoggerPtr_ = std::make_shared<trantor::AsyncFileLogger>();
             asyncFileLoggerPtr_->setFileName(baseName, ".log", logPath_);
             asyncFileLoggerPtr_->startLogging();
-            trantor::Logger::setOutputFunction(
-                [this](const char *msg, const uint64_t len) {
-                    asyncFileLoggerPtr_->output(msg, len);
-                },
-                [this]() { asyncFileLoggerPtr_->flush(); });
             asyncFileLoggerPtr_->setFileSizeLimit(logfileSize_);
+            trantor::Logger::setOutputFunction(
+                [loggerPtr = asyncFileLoggerPtr_](const char *msg,
+                                                  const uint64_t len) {
+                    loggerPtr->output(msg, len);
+                },
+                [loggerPtr = asyncFileLoggerPtr_]() { loggerPtr->flush(); });
         }
     }
     return *this;
diff --git a/lib/src/HttpAppFrameworkImpl.h b/lib/src/HttpAppFrameworkImpl.h
index b2363931..de68c7a1 100644
--- a/lib/src/HttpAppFrameworkImpl.h
+++ b/lib/src/HttpAppFrameworkImpl.h
@@ -588,16 +588,16 @@ class HttpAppFrameworkImpl final : public HttpAppFramework
     std::string serverHeader_{"server: drogon/" + drogon::getVersion() +
                               "\r\n"};
 
-    const std::unique_ptr<StaticFileRouter> staticFileRouterPtr_;
-    const std::unique_ptr<HttpControllersRouter> httpCtrlsRouterPtr_;
-    const std::unique_ptr<HttpSimpleControllersRouter>
-        httpSimpleCtrlsRouterPtr_;
-    const std::unique_ptr<WebsocketControllersRouter> websockCtrlsRouterPtr_;
+    std::unique_ptr<StaticFileRouter> staticFileRouterPtr_;
+    std::unique_ptr<HttpControllersRouter> httpCtrlsRouterPtr_;
+    std::unique_ptr<HttpSimpleControllersRouter> httpSimpleCtrlsRouterPtr_;
+    std::unique_ptr<WebsocketControllersRouter> websockCtrlsRouterPtr_;
+
+    std::unique_ptr<ListenerManager> listenerManagerPtr_;
+    std::unique_ptr<PluginsManager> pluginsManagerPtr_;
+    std::unique_ptr<orm::DbClientManager> dbClientManagerPtr_;
+    std::unique_ptr<nosql::RedisClientManager> redisClientManagerPtr_;
 
-    const std::unique_ptr<ListenerManager> listenerManagerPtr_;
-    const std::unique_ptr<PluginsManager> pluginsManagerPtr_;
-    const std::unique_ptr<orm::DbClientManager> dbClientManagerPtr_;
-    const std::unique_ptr<nosql::RedisClientManager> redisClientManagerPtr_;
     std::string rootPath_{"./"};
     std::string uploadPath_;
     std::atomic_bool running_{false};
@@ -641,7 +641,7 @@ class HttpAppFrameworkImpl final : public HttpAppFramework
     std::function<void()> termSignalHandler_{[]() { app().quit(); }};
     std::function<void()> intSignalHandler_{[]() { app().quit(); }};
     std::unique_ptr<SessionManager> sessionManagerPtr_;
-    std::unique_ptr<trantor::AsyncFileLogger> asyncFileLoggerPtr_;
+    std::shared_ptr<trantor::AsyncFileLogger> asyncFileLoggerPtr_;
     Json::Value jsonConfig_;
     HttpResponsePtr custom404_;
     std::function<HttpResponsePtr(HttpStatusCode)> customErrorHandler_ =
diff --git a/lib/src/ListenerManager.cc b/lib/src/ListenerManager.cc
index af2af42a..f9fe47f5 100644
--- a/lib/src/ListenerManager.cc
+++ b/lib/src/ListenerManager.cc
@@ -268,20 +268,6 @@ void ListenerManager::stopListening()
     {
         serverPtr->stop();
     }
-    for (auto loop : ioLoops_)
-    {
-        assert(!loop->isInLoopThread());
-        if (loop->isRunning())
-        {
-            std::promise<int> pro;
-            auto f = pro.get_future();
-            loop->queueInLoop([loop, &pro]() {
-                loop->quit();
-                pro.set_value(1);
-            });
-            (void)f.get();
-        }
-    }
 #ifndef __linux__
     for (auto &listenerLoopPtr : listeningloopThreads_)
     {
@@ -301,6 +287,24 @@ void ListenerManager::stopListening()
 #endif
 }
 
+void ListenerManager::stopIoLoops()
+{
+    for (auto loop : ioLoops_)
+    {
+        assert(!loop->isInLoopThread());
+        if (loop->isRunning())
+        {
+            std::promise<int> pro;
+            auto f = pro.get_future();
+            loop->queueInLoop([loop, &pro]() {
+                loop->quit();
+                pro.set_value(1);
+            });
+            (void)f.get();
+        }
+    }
+}
+
 std::vector<trantor::InetAddress> ListenerManager::getListeners() const
 {
     std::vector<trantor::InetAddress> listeners;
diff --git a/lib/src/ListenerManager.h b/lib/src/ListenerManager.h
index 950748eb..6cba677b 100644
--- a/lib/src/ListenerManager.h
+++ b/lib/src/ListenerManager.h
@@ -59,6 +59,7 @@ class ListenerManager : public trantor::NonCopyable
 
     trantor::EventLoop *getIOLoop(size_t id) const;
     void stopListening();
+    void stopIoLoops();
     std::vector<trantor::EventLoop *> ioLoops_;
 
   private:
diff --git a/lib/src/StaticFileRouter.cc b/lib/src/StaticFileRouter.cc
index 7617dec5..8be0d6c2 100644
--- a/lib/src/StaticFileRouter.cc
+++ b/lib/src/StaticFileRouter.cc
@@ -37,8 +37,8 @@ using namespace drogon;
 void StaticFileRouter::init(const std::vector<trantor::EventLoop *> &ioloops)
 {
     // Max timeout up to about 70 days;
-    staticFilesCacheMap_ = decltype(staticFilesCacheMap_)(
-        new IOThreadStorage<std::unique_ptr<CacheMap<std::string, char>>>);
+    staticFilesCacheMap_ = std::make_unique<
+        IOThreadStorage<std::unique_ptr<CacheMap<std::string, char>>>>();
     staticFilesCacheMap_->init(
         [&ioloops](std::unique_ptr<CacheMap<std::string, char>> &mapPtr,
                    size_t i) {
@@ -46,14 +46,16 @@ void StaticFileRouter::init(const std::vector<trantor::EventLoop *> &ioloops)
             mapPtr = std::unique_ptr<CacheMap<std::string, char>>(
                 new CacheMap<std::string, char>(ioloops[i], 1.0, 4, 50));
         });
-    staticFilesCache_ = decltype(staticFilesCache_)(
-        new IOThreadStorage<
-            std::unordered_map<std::string, HttpResponsePtr>>{});
+    staticFilesCache_ = std::make_unique<
+        IOThreadStorage<std::unordered_map<std::string, HttpResponsePtr>>>();
     ioLocationsPtr_ =
-        decltype(ioLocationsPtr_)(new IOThreadStorage<std::vector<Location>>);
+        std::make_shared<IOThreadStorage<std::vector<Location>>>();
     for (auto *loop : ioloops)
     {
-        loop->queueInLoop([this] { **ioLocationsPtr_ = locations_; });
+        loop->queueInLoop(
+            [ioLocationsPtr = ioLocationsPtr_, locations = locations_] {
+                **ioLocationsPtr = locations;
+            });
     }
 }
 
diff --git a/lib/src/StaticFileRouter.h b/lib/src/StaticFileRouter.h
index cce08950..11e8c353 100644
--- a/lib/src/StaticFileRouter.h
+++ b/lib/src/StaticFileRouter.h
@@ -169,7 +169,7 @@ class StaticFileRouter
             }
         }
     };
-    std::unique_ptr<IOThreadStorage<std::vector<Location>>> ioLocationsPtr_;
+    std::shared_ptr<IOThreadStorage<std::vector<Location>>> ioLocationsPtr_;
     std::vector<Location> locations_;
 };
 }  // namespace drogon