src: fix monerod shutdown hanging by making SIGINT handling async-signal-safe

Author: navidR

contrib/epee/include/net/abstract_tcp_server2.inl

@@ -1706,26 +1706,40 @@ namespace net_utils
     boost::unique_lock<boost::mutex> lock(local_shared_context->connect_mut);
     auto connect_callback = [](boost::system::error_code ec_, boost::shared_ptr<local_async_context> shared_context)
     {
-      shared_context->connect_mut.lock(); shared_context->ec = ec_; shared_context->cond.notify_one(); shared_context->connect_mut.unlock();
+      boost::lock_guard<boost::mutex> callback_lock(shared_context->connect_mut);
+      shared_context->ec = ec_;
+      shared_context->cond.notify_one();
     };
 
     sock_.async_connect(remote_endpoint, std::bind<void>(connect_callback, std::placeholders::_1, local_shared_context));
+    const auto deadline = boost::get_system_time() + boost::posix_time::milliseconds(conn_timeout);
     while(local_shared_context->ec == boost::asio::error::would_block)
     {
-      bool r = local_shared_context->cond.timed_wait(lock, boost::get_system_time() + boost::posix_time::milliseconds(conn_timeout));
       if (m_stop_signal_sent)
       {
+        boost::system::error_code ignored_ec;
+        sock_.cancel(ignored_ec);
         if (sock_.is_open())
           sock_.close();
         return CONNECT_FAILURE;
       }
-      if(local_shared_context->ec == boost::asio::error::would_block && !r)
+      const auto now = boost::get_system_time();
+      if (now >= deadline)
       {
-        //timeout
+        // timeout
         sock_.close();
         _dbg3("Failed to connect to " << adr << ":" << port << ", because of timeout (" << conn_timeout << ")");
         return CONNECT_FAILURE;
       }
+      const auto remaining = deadline - now;
+      const auto wait_step = remaining < boost::posix_time::milliseconds(100)
+        ? remaining
+        : boost::posix_time::milliseconds(100);
+      bool r = local_shared_context->cond.timed_wait(lock, now + wait_step);
+      if(local_shared_context->ec == boost::asio::error::would_block && !r)
+      {
+        continue;
+      }
     }
     ec = local_shared_context->ec;
 

src/common/util.cpp

@@ -31,6 +31,7 @@
 #include <unistd.h>
 #include <cstdio>
 #include <wchar.h>
+#include <cerrno>
 
 #ifdef __GLIBC__
 #include <gnu/libc-version.h>
@@ -79,6 +80,7 @@ using namespace epee;
 #else 
   #include <sys/file.h>
   #include <sys/stat.h>
+  #include <fcntl.h>
 #endif
 #include <boost/filesystem.hpp>
 #include <boost/algorithm/string.hpp>
@@ -133,6 +135,73 @@ namespace tools
   }
 
   std::function<void(int)> signal_handler::m_handler;
+#ifndef WIN32
+  std::atomic<bool> signal_handler::m_dispatch_thread_started{false};
+  std::thread signal_handler::m_dispatch_thread;
+  int signal_handler::m_signal_pipe_read = -1;
+  int signal_handler::m_signal_pipe_write = -1;
+
+  bool signal_handler::install_posix()
+  {
+    static boost::mutex install_mutex;
+    boost::lock_guard<boost::mutex> install_lock(install_mutex);
+
+    if (!m_dispatch_thread_started.load(std::memory_order_acquire))
+    {
+      int pipe_fds[2]{-1, -1};
+      if (pipe(pipe_fds) != 0)
+      {
+        MERROR("Failed to create signal pipe: " << errno);
+        return false;
+      }
+
+      // Make write end non-blocking so signal handler never blocks.
+      const int flags = fcntl(pipe_fds[1], F_GETFL, 0);
+      if (flags >= 0)
+        fcntl(pipe_fds[1], F_SETFL, flags | O_NONBLOCK);
+      fcntl(pipe_fds[0], F_SETFD, FD_CLOEXEC);
+      fcntl(pipe_fds[1], F_SETFD, FD_CLOEXEC);
+
+      m_signal_pipe_read = pipe_fds[0];
+      m_signal_pipe_write = pipe_fds[1];
+      m_dispatch_thread = std::thread(&signal_handler::signal_dispatch_thread);
+      m_dispatch_thread.detach();
+      m_dispatch_thread_started.store(true, std::memory_order_release);
+    }
+
+    static struct sigaction sa;
+    memset(&sa, 0, sizeof(struct sigaction));
+    sa.sa_handler = posix_handler;
+    sa.sa_flags = 0;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGINT, &sa, NULL);
+    sigaction(SIGTERM, &sa, NULL);
+    signal(SIGPIPE, SIG_IGN);
+    return true;
+  }
+
+  void signal_handler::signal_dispatch_thread()
+  {
+    for (;;)
+    {
+      uint8_t signal_byte = 0;
+      const ssize_t bytes = read(m_signal_pipe_read, &signal_byte, sizeof(signal_byte));
+      if (bytes == sizeof(signal_byte))
+      {
+        handle_signal(static_cast<int>(signal_byte));
+      }
+      else if (bytes < 0 && errno == EINTR)
+      {
+        continue;
+      }
+      else
+      {
+        // Prevent spin on pipe failures.
+        epee::misc_utils::sleep_no_w(50);
+      }
+    }
+  }
+#endif
 
   private_file::private_file() noexcept : m_handle(), m_filename() {}
 

src/common/util.h

@@ -39,6 +39,12 @@
 #include <functional>
 #include <memory>
 #include <string>
+#include <thread>
+#include <atomic>
+#include <cstdint>
+#ifndef WIN32
+#include <unistd.h>
+#endif
 
 #ifdef _WIN32
 #include "windows.h"
@@ -170,23 +176,14 @@ namespace tools
     static bool install(T t)
     {
 #if defined(WIN32)
-      bool r = TRUE == ::SetConsoleCtrlHandler(&win_handler, TRUE);
-      if (r)
-      {
-        m_handler = t;
-      }
+      m_handler = t;
+      const bool r = TRUE == ::SetConsoleCtrlHandler(&win_handler, TRUE);
+      if (!r)
+        m_handler = {};
       return r;
 #else
-      static struct sigaction sa;
-      memset(&sa, 0, sizeof(struct sigaction));
-      sa.sa_handler = posix_handler;
-      sa.sa_flags = 0;
-      /* Only blocks SIGINT, SIGTERM and SIGPIPE */
-      sigaction(SIGINT, &sa, NULL);
-      signal(SIGTERM, posix_handler);
-      signal(SIGPIPE, SIG_IGN);
       m_handler = t;
-      return true;
+      return install_posix();
 #endif
     }
 
@@ -210,20 +207,38 @@ namespace tools
     /*! \brief handler for NIX */
     static void posix_handler(int type)
     {
-      handle_signal(type);
+      // Async-signal-safe: enqueue the signal number for a normal thread.
+      const uint8_t signal_byte = static_cast<uint8_t>(type);
+      int ignored = ::write(m_signal_pipe_write, &signal_byte, sizeof(signal_byte));
+      (void)ignored;
     }
 #endif
 
     /*! \brief calles m_handler */
     static void handle_signal(int type)
     {
+#ifdef WIN32
       static boost::mutex m_mutex;
       boost::unique_lock<boost::mutex> lock(m_mutex);
-      m_handler(type);
+#endif
+      if (m_handler)
+        m_handler(type);
     }
 
+#ifndef WIN32
+    static bool install_posix();
+    static void signal_dispatch_thread();
+#endif
+
     /*! \brief where the installed handler is stored */
     static std::function<void(int)> m_handler;
+
+#ifndef WIN32
+    static std::atomic<bool> m_dispatch_thread_started;
+    static std::thread m_dispatch_thread;
+    static int m_signal_pipe_read;
+    static int m_signal_pipe_write;
+#endif
   };
 
   void set_strict_default_file_permissions(bool strict);

src/p2p/net_node.inl

@@ -1160,6 +1160,10 @@ namespace nodetool
         zone.second.m_net_server.get_config_object().close(connection_id);
     }
     m_payload_handler.stop();
+    // Join worker threads quickly on shutdown; if some are still stuck in
+    // connect/cancel paths, timed_wait_server_stop() will interrupt them.
+    for (auto& zone : m_network_zones)
+      zone.second.m_net_server.timed_wait_server_stop(2000);
     return true;
   }
   //-----------------------------------------------------------------------------------