MB-58609: NIM needs to run in its own thread

Previously we used the "main thead" of the program to accept
clients, reconfigure network interfaces etc. This is problematic
during shutdown as we terminate the event loop for the main
thread and initiate shutdown for the other components. As part
of shutting down we need to stop all of the worker threads,
but we might have a command currently trying to run network
reconfigure. This will hang "forever" as we've stopped running
the event dispatch loop.

Change-Id: Ieebe65e2b542d9878b9faaf01b832c87cc7df094
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/197098
Tested-by: Build Bot <[email protected]>
Reviewed-by: Jim Walker <[email protected]>

Author: trondn

daemon/CMakeLists.txt

@@ -64,6 +64,8 @@ add_library(memcached_daemon STATIC
             network_interface_description.h
             network_interface_manager.cc
             network_interface_manager.h
+            network_interface_manager_thread.cc
+            network_interface_manager_thread.h
             nobucket_taskable.cc
             nobucket_taskable.h
             resource_allocation_domain.cc

daemon/memcached.cc

@@ -25,7 +25,7 @@
 #include "mcaudit.h"
 #include "mcbp_executors.h"
 #include "network_interface.h"
-#include "network_interface_manager.h"
+#include "network_interface_manager_thread.h"
 #include "nobucket_taskable.h"
 #include "protocol/mcbp/engine_wrapper.h"
 #include "settings.h"
@@ -1005,12 +1005,8 @@ int memcached_main(int argc, char** argv) {
     }
 #endif
 
-    LOG_INFO_RAW("Starting network interface manager");
-    networkInterfaceManager = std::make_unique<NetworkInterfaceManager>(
-            *main_base, prometheus_auth_callback);
-    networkInterfaceManager->createBootstrapInterface();
-
-    /* start up worker threads if MT mode */
+    auto nim_thread = std::make_unique<NetworkInterfaceManagerThread>(
+            prometheus_auth_callback);
     worker_threads_init();
 
     LOG_INFO(R"(Starting Phosphor tracing with config: "{}")",
@@ -1051,9 +1047,10 @@ int memcached_main(int argc, char** argv) {
         }
     }
 
+    LOG_INFO_RAW("Initialization complete. Accepting clients.");
+    nim_thread->start();
+
     if (!memcached_shutdown) {
-        /* enter the event loop */
-        LOG_INFO_RAW("Initialization complete. Accepting clients.");
         main_base->loopForever();
     }
 
@@ -1073,8 +1070,10 @@ int memcached_main(int argc, char** argv) {
     LOG_INFO_RAW("Shutting down client worker threads");
     threads_shutdown();
 
-    LOG_INFO_RAW("Releasing server sockets");
-    networkInterfaceManager.reset();
+    LOG_INFO_RAW("Shutting down network interface manager thread");
+    nim_thread->shutdown();
+    nim_thread->waitForState(Couchbase::ThreadState::Zombie);
+    nim_thread.reset();
 
     LOG_INFO_RAW("Releasing bucket resources");
     cleanup_buckets();

daemon/network_interface_manager_thread.cc

@@ -0,0 +1,41 @@
+/*
+ *     Copyright 2023-Present Couchbase, Inc.
+ *
+ *   Use of this software is governed by the Business Source License included
+ *   in the file licenses/BSL-Couchbase.txt.  As of the Change Date specified
+ *   in that file, in accordance with the Business Source License, use of this
+ *   software will be governed by the Apache License, Version 2.0, included in
+ *   the file licenses/APL2.txt.
+ */
+#include "network_interface_manager_thread.h"
+#include "network_interface_manager.h"
+
+#include "log_macros.h"
+
+NetworkInterfaceManagerThread::NetworkInterfaceManagerThread(
+        cb::prometheus::AuthCallback authCB)
+    : Couchbase::Thread("mcd:nim"), base(std::make_unique<folly::EventBase>()) {
+    LOG_INFO_RAW("Starting network interface manager");
+    networkInterfaceManager =
+            std::make_unique<NetworkInterfaceManager>(*base, std::move(authCB));
+    networkInterfaceManager->createBootstrapInterface();
+}
+
+void NetworkInterfaceManagerThread::run() {
+    setRunning();
+    try {
+        base->loopForever();
+    } catch (const std::exception& exception) {
+        LOG_ERROR(
+                "NetworkInterfaceManagerThread::run(): received exception: {}",
+                exception.what());
+    }
+
+    LOG_INFO_RAW("Releasing server sockets");
+    networkInterfaceManager.reset();
+    LOG_INFO_RAW("Network interface manager thread stopped");
+}
+
+void NetworkInterfaceManagerThread::shutdown() {
+    base->terminateLoopSoon();
+}

daemon/network_interface_manager_thread.h

@@ -0,0 +1,48 @@
+/*
+ *     Copyright 2023-Present Couchbase, Inc.
+ *
+ *   Use of this software is governed by the Business Source License included
+ *   in the file licenses/BSL-Couchbase.txt.  As of the Change Date specified
+ *   in that file, in accordance with the Business Source License, use of this
+ *   software will be governed by the Apache License, Version 2.0, included in
+ *   the file licenses/APL2.txt.
+ */
+
+#pragma once
+
+#include <folly/io/async/EventBase.h>
+#include <platform/thread.h>
+#include <statistics/prometheus.h>
+#include <memory>
+
+/**
+ * The NetworkInterfaceManagerThread class runs all of the code
+ * in the network manager (accept clients, reconfigure interface
+ * etc). It needs to be run in its own thread as we clients may
+ * be performing an operation which touch this code during
+ * shutdown where the "main thread" has stopped the event loop
+ * and is stopping other components.
+ */
+class NetworkInterfaceManagerThread : public Couchbase::Thread {
+public:
+    /**
+     * Create a new thread which use the provided callback for
+     * prometheus authentication
+     *
+     * @param authCB callback performing authentication
+     */
+    NetworkInterfaceManagerThread(cb::prometheus::AuthCallback authCB);
+    NetworkInterfaceManagerThread() = delete;
+    NetworkInterfaceManagerThread(const NetworkInterfaceManagerThread&) =
+            delete;
+
+    /// Request the thread to stop
+    void shutdown();
+
+protected:
+    /// The main loop of the thread
+    void run() override;
+
+    /// The event base used for this thread
+    std::unique_ptr<folly::EventBase> base;
+};

daemon/server_socket.cc

@@ -109,16 +109,6 @@ void LibeventServerSocketImpl::listen_event_handler(evutil_socket_t,
                                                     short,
                                                     void* arg) {
     auto& c = *reinterpret_cast<LibeventServerSocketImpl*>(arg);
-
-    if (is_memcached_shutting_down()) {
-        // Someone requested memcached to shut down. The listen thread should
-        // be stopped immediately to avoid new connections
-        LOG_INFO_RAW("Stopping listen thread");
-        event_del(c.ev.get());
-        c.ev.reset();
-        return;
-    }
-
     try {
         c.acceptNewClient();
     } catch (std::invalid_argument& e) {
@@ -225,6 +215,11 @@ void LibeventServerSocketImpl::acceptNewClient() {
         return;
     }
 
+    if (is_memcached_shutting_down()) {
+        safe_close(client);
+        return;
+    }
+
     if (interface->tls && !tls_configured) {
         tls_configured = networkInterfaceManager->isTlsConfigured();
         if (!tls_configured) {