From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Mon, 19 Apr 2021 14:32:05 +0000 (+0000)
Subject: crimson/common: don't use Seastar's handle_signal() for fatal signals.
X-Git-Tag: v17.1.0~2192^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=33cc480d1182412ee00be527ac033bb5515ce023;p=ceph.git

crimson/common: don't use Seastar's handle_signal() for fatal signals.

The `handle_signal()` way requires going through the reactor, and
thus is unsuitable for situations when it's no longer operational.
SIGSEGV generated as a result of actual fault (not `kill()`) is
the intended audience of this change.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/crimson/common/fatal_signal.cc b/src/crimson/common/fatal_signal.cc
index 55988d903fe3..a03259a86820 100644
--- a/src/crimson/common/fatal_signal.cc
+++ b/src/crimson/common/fatal_signal.cc
@@ -3,7 +3,7 @@
 
 #include "fatal_signal.h"
 
-#include <signal.h>
+#include <csignal>
 #include <iostream>
 #include <string_view>
 
@@ -11,22 +11,44 @@
 
 FatalSignal::FatalSignal()
 {
-  auto fatal_signals = {SIGSEGV,
-                        SIGABRT,
-                        SIGBUS,
-                        SIGILL,
-                        SIGFPE,
-                        SIGXCPU,
-                        SIGXFSZ,
-                        SIGSYS};
-
-  for (auto signum : fatal_signals) {
-    seastar::engine().handle_signal(signum, [signum, this] {
-      signaled(signum);
-    });
+  install_oneshot_signals_handler<SIGSEGV,
+                                  SIGABRT,
+                                  SIGBUS,
+                                  SIGILL,
+                                  SIGFPE,
+                                  SIGXCPU,
+                                  SIGXFSZ,
+                                  SIGSYS>();
+}
+
+template <int... SigNums>
+void FatalSignal::install_oneshot_signals_handler()
+{
+  (install_oneshot_signal_handler<SigNums>() , ...);
+}
+
+template <int SigNum>
+void FatalSignal::install_oneshot_signal_handler()
+{
+  struct sigaction sa;
+  sa.sa_sigaction = [](int sig, siginfo_t *info, void *p) {
+    static std::atomic_bool handled = false;
+    if (handled.exchange(true)) {
+      return;
+    }
+    FatalSignal::signaled(sig);
+    ::signal(sig, SIG_DFL);
+  };
+  sigfillset(&sa.sa_mask);
+  sa.sa_flags = SA_SIGINFO | SA_RESTART;
+  if constexpr (SigNum == SIGSEGV) {
+      sa.sa_flags |= SA_ONSTACK;
   }
+  [[maybe_unused]] auto r = ::sigaction(SigNum, &sa, nullptr);
+  assert(r);
 }
 
+
 static void print_with_backtrace(std::string_view cause) {
   std::cerr << cause;
   if (seastar::engine_is_ready()) {
@@ -43,9 +65,6 @@ static void print_with_backtrace(std::string_view cause) {
 
 void FatalSignal::signaled(int signum)
 {
-  if (handled.exchange(true)) {
-    return;
-  }
   switch (signum) {
   case SIGSEGV:
     print_with_backtrace("Aborting");
@@ -57,5 +76,4 @@ void FatalSignal::signaled(int signum)
     print_with_backtrace(fmt::format("Signal {}", signum));
     break;
   }
-  signal(signum, SIG_DFL);
 }
diff --git a/src/crimson/common/fatal_signal.h b/src/crimson/common/fatal_signal.h
index 8320f99d6905..158b87f0c768 100644
--- a/src/crimson/common/fatal_signal.h
+++ b/src/crimson/common/fatal_signal.h
@@ -3,14 +3,17 @@
 
 #pragma once
 
-#include <atomic>
-
 class FatalSignal {
 public:
   FatalSignal();
 
 private:
-  void signaled(int signum);
+  static void signaled(int signum);
   static void print_backtrace(int signum);
-  std::atomic<bool> handled = false;
+
+  template <int... SigNums>
+  void install_oneshot_signals_handler();
+
+  template <int SigNum>
+  void install_oneshot_signal_handler();
 };