From 1514a773459d2bbbee197a234397b87db5787d18 Mon Sep 17 00:00:00 2001
From: Esa Korhonen <esa.korhonen@mariadb.com>
Date: Thu, 6 Jun 2019 10:17:17 +0300
Subject: [PATCH] Improve main thread detection

Now properly checks if configuration and diagnostics functions are ran in either
main() or in the admin worker. This is useful for debugging and enforcing
thread safety.

Also, monitors are now started and stopped in the admin worker.
---
 server/core/gateway.cc           | 28 +++++++++++++++-------------
 server/core/internal/maxscale.hh |  6 +++---
 server/core/misc.cc              | 26 ++++++++++++++++++++++++++
 server/core/monitor.cc           |  4 ++--
 4 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/server/core/gateway.cc b/server/core/gateway.cc
index 3103b8100..fa845013e 100644
--- a/server/core/gateway.cc
+++ b/server/core/gateway.cc
@@ -1442,10 +1442,15 @@ int main(int argc, char** argv)
                 rc = MAXSCALE_NOSERVICES;
                 maxscale_shutdown();
             }
-            else if (daemon_mode)
+            else
             {
-                // Successful start, notify the parent process that it can exit.
-                write_child_exit_code(daemon_pipe[1], rc);
+                if (daemon_mode)
+                {
+                    // Successful start, notify the parent process that it can exit.
+                    write_child_exit_code(daemon_pipe[1], rc);
+                }
+                /** Start all monitors */
+                MonitorManager::start_all_monitors();
             }
         };
 
@@ -2188,9 +2193,6 @@ int main(int argc, char** argv)
         goto return_main;
     }
 
-    /** Start all monitors */
-    MonitorManager::start_all_monitors();
-
     if (cnf->config_check)
     {
         MXS_NOTICE("Configuration was successfully verified.");
@@ -2247,6 +2249,8 @@ int main(int argc, char** argv)
     worker = RoutingWorker::get(RoutingWorker::MAIN);
     mxb_assert(worker);
 
+    // Configuration read and items created. Changes should now come through the main routing worker.
+    set_admin_worker(worker);
     if (!worker->execute(do_startup, RoutingWorker::EXECUTE_QUEUED))
     {
         const char* logerr = "Failed to queue startup task.";
@@ -2257,19 +2261,17 @@ int main(int argc, char** argv)
 
     main_worker->run();
 
-    /*< Stop all monitors */
-    MonitorManager::stop_all_monitors();
-
-    /*< Destroy all monitors */
-    MonitorManager::destroy_all_monitors();
-
     /*<
      * Wait for worker threads to exit.
      */
     RoutingWorker::join_workers();
-
     MXS_NOTICE("All workers have shut down.");
 
+    set_admin_worker(nullptr); // Main worker has quit, re-assign to non-worker.
+
+    /*< Destroy all monitors */
+    MonitorManager::destroy_all_monitors();
+
     maxscale_start_teardown();
 
     /*<
diff --git a/server/core/internal/maxscale.hh b/server/core/internal/maxscale.hh
index fc6154cd0..b6b60a572 100644
--- a/server/core/internal/maxscale.hh
+++ b/server/core/internal/maxscale.hh
@@ -18,8 +18,6 @@
 
 #include <maxscale/maxscale.h>
 
-MXS_BEGIN_DECLS
-
 /**
  * Initiate shutdown of MaxScale.
  *
@@ -39,4 +37,6 @@ void maxscale_reset_starttime(void);
 bool maxscale_teardown_in_progress();
 void maxscale_start_teardown();
 
-MXS_END_DECLS
+bool running_in_admin_thread();
+
+void set_admin_worker(const mxb::Worker* admin_worker);
diff --git a/server/core/misc.cc b/server/core/misc.cc
index d410b75f0..f5f942650 100644
--- a/server/core/misc.cc
+++ b/server/core/misc.cc
@@ -11,6 +11,7 @@
  * Public License.
  */
 
+#include <maxscale/ccdefs.hh>
 #include <maxscale/maxscale.h>
 
 #include <time.h>
@@ -21,9 +22,19 @@
 #include "internal/maxscale.hh"
 #include "internal/service.hh"
 #include "internal/admin.hh"
+#include "internal/monitormanager.hh"
 
 static time_t started;
 
+namespace
+{
+    struct ThisUnit
+    {
+        std::atomic<const mxb::Worker*> admin_worker {nullptr};
+    };
+    ThisUnit this_unit;
+}
+
 void maxscale_reset_starttime(void)
 {
     started = time(0);
@@ -58,6 +69,9 @@ int maxscale_shutdown()
                     mxs::MainWorker::get().shutdown();
                 }
 
+                /*< Stop all monitors */
+                MonitorManager::stop_all_monitors();
+
                 mxs_admin_shutdown();
                 mxs::RoutingWorker::shutdown_all();
             };
@@ -80,3 +94,15 @@ void maxscale_start_teardown()
 {
     teardown_in_progress = true;
 }
+
+bool running_in_admin_thread()
+{
+    auto current_worker = mxb::Worker::get_current();
+    return current_worker == this_unit.admin_worker.load(std::memory_order_acquire);
+}
+
+void set_admin_worker(const mxb::Worker* admin_worker)
+{
+    this_unit.admin_worker.store(admin_worker, std::memory_order_release);
+}
+
diff --git a/server/core/monitor.cc b/server/core/monitor.cc
index 75edf1779..c6635ac12 100644
--- a/server/core/monitor.cc
+++ b/server/core/monitor.cc
@@ -46,6 +46,7 @@
 
 #include "internal/config.hh"
 #include "internal/externcmd.hh"
+#include "internal/maxscale.hh"
 #include "internal/monitor.hh"
 #include "internal/modules.hh"
 #include "internal/server.hh"
@@ -1212,8 +1213,7 @@ string Monitor::get_server_monitor(const SERVER* server)
 
 bool Monitor::is_admin_thread()
 {
-    mxb::Worker* current = mxb::Worker::get_current();
-    return current == nullptr || current == mxs_rworker_get(MXS_RWORKER_MAIN);
+    return running_in_admin_thread();
 }
 
 /**