You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2014/08/28 01:17:23 UTC

[3/3] git commit: Fixed signal handling for Mesos.

Fixed signal handling for Mesos.

Review: https://reviews.apache.org/r/25129


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6af54321
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6af54321
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6af54321

Branch: refs/heads/master
Commit: 6af543211d7c35e783fb8b23341810fa678fb78a
Parents: 0386b6e
Author: Vinod Kone <vi...@gmail.com>
Authored: Wed Aug 27 15:47:58 2014 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Wed Aug 27 15:54:41 2014 -0700

----------------------------------------------------------------------
 src/logging/logging.cpp | 67 ++++++++++++++++++++++++++++++++++++++++----
 src/tests/main.cpp      |  9 ++++++
 2 files changed, 71 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6af54321/src/logging/logging.cpp
----------------------------------------------------------------------
diff --git a/src/logging/logging.cpp b/src/logging/logging.cpp
index 547673c..6b14575 100644
--- a/src/logging/logging.cpp
+++ b/src/logging/logging.cpp
@@ -28,12 +28,13 @@
 
 #include <stout/error.hpp>
 #include <stout/exit.hpp>
-#include <stout/glog.hpp>
 #include <stout/os.hpp>
 #include <stout/path.hpp>
 #include <stout/stringify.hpp>
 #include <stout/try.hpp>
 
+#include <stout/os/signals.hpp>
+
 #include "logging/logging.hpp"
 
 using process::Once;
@@ -66,7 +67,36 @@ namespace logging {
 string argv0;
 
 
-google::LogSeverity getLogSeverity(const std::string& logging_level)
+// NOTE: We use RAW_LOG instead of LOG because RAW_LOG doesn't
+// allocate any memory or grab locks. And according to
+// https://code.google.com/p/google-glog/issues/detail?id=161
+// it should work in 'most' cases in signal handlers.
+inline void handler(int signal, siginfo_t *siginfo, void *context)
+{
+  if (signal == SIGTERM) {
+    if (siginfo->si_code == SI_USER ||
+        siginfo->si_code == SI_QUEUE ||
+        siginfo->si_code <= 0) {
+      RAW_LOG(WARNING, "Received signal SIGTERM from process %d of user %d; "
+                       "exiting", siginfo->si_pid, siginfo->si_uid);
+    } else {
+      RAW_LOG(WARNING, "Received signal SIGTERM; exiting");
+    }
+
+    // Setup the default handler for SIGTERM so that we don't print
+    // a stack trace.
+    os::signals::reset(signal);
+    raise(signal);
+  } else if (signal == SIGPIPE) {
+    RAW_LOG(WARNING, "Received signal SIGPIPE; escalating to SIGABRT");
+    raise(SIGABRT);
+  } else {
+    RAW_LOG(FATAL, "Unexpected signal in signal handler: %d", signal);
+  }
+}
+
+
+google::LogSeverity getLogSeverity(const string& logging_level)
 {
   if (logging_level == "INFO") {
     return google::INFO;
@@ -84,7 +114,7 @@ google::LogSeverity getLogSeverity(const std::string& logging_level)
 void initialize(
     const string& _argv0,
     const Flags& flags,
-    bool _installFailureSignalHandler)
+    bool installFailureSignalHandler)
 {
   static Once* initialized = new Once();
 
@@ -147,8 +177,35 @@ void initialize(
   VLOG(1) << "Logging to " <<
     (flags.log_dir.isSome() ? flags.log_dir.get() : "STDERR");
 
-  if (_installFailureSignalHandler) {
-    installFailureSignalHandler();
+  if (installFailureSignalHandler) {
+    // Handles SIGSEGV, SIGILL, SIGFPE, SIGABRT, SIGBUS, SIGTERM
+    // by default.
+    google::InstallFailureSignalHandler();
+
+    // Set up our custom signal handlers.
+    struct sigaction action;
+    action.sa_sigaction = handler;
+
+    // Do not block additional signals while in the handler.
+    sigemptyset(&action.sa_mask);
+
+    // The SA_SIGINFO flag tells sigaction() to use
+    // the sa_sigaction field, not sa_handler.
+    action.sa_flags = SA_SIGINFO;
+
+    // Set up the SIGPIPE signal handler to escalate to SIGABRT
+    // in order to have the glog handler catch it and print all
+    // of its lovely information.
+    if (sigaction(SIGPIPE, &action, NULL) < 0) {
+      PLOG(FATAL) << "Failed to set sigaction";
+    }
+
+    // We also do not want SIGTERM to dump a stacktrace, as this
+    // can imply that we crashed, when we were in fact terminated
+    // by user request.
+    if (sigaction(SIGTERM, &action, NULL) < 0) {
+      PLOG(FATAL) << "Failed to set sigaction";
+    }
   }
 
   initialized->done();

http://git-wip-us.apache.org/repos/asf/mesos/blob/6af54321/src/tests/main.cpp
----------------------------------------------------------------------
diff --git a/src/tests/main.cpp b/src/tests/main.cpp
index 32a2456..42b1a58 100644
--- a/src/tests/main.cpp
+++ b/src/tests/main.cpp
@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+#include <signal.h>
+
 #include <gtest/gtest.h>
 
 #include <string>
@@ -27,6 +29,8 @@
 #include <stout/os.hpp>
 #include <stout/try.hpp>
 
+#include <stout/os/signals.hpp>
+
 #include "logging/logging.hpp"
 
 #include "messages/messages.hpp" // For GOOGLE_PROTOBUF_VERIFY_VERSION.
@@ -91,6 +95,11 @@ int main(int argc, char** argv)
   // Initialize logging.
   logging::initialize(argv[0], flags, true);
 
+  // We reset SIGPIPE default handler because some Mesos tests
+  // run an in-process ZooKeeper which results in SIGPIPE during
+  // ZooKeeeper server shutdown. See MESOS-1729 for details.
+  os::signals::reset(SIGPIPE);
+
   // Initialize gmock/gtest.
   testing::InitGoogleTest(&argc, argv);
   testing::FLAGS_gtest_death_test_style = "threadsafe";