Merge pull request #3685 from vespa-engine/arnej/refactor-sig-catch

Arnej/refactor sig catch

17 files changed, 89 insertions, 151 deletions

diff --git a/configd/src/apps/sentinel/sentinel.cpp b/configd/src/apps/sentinel/sentinel.cpp
index 45bbbe19cf3..bb05f9e40ad 100644
--- a/configd/src/apps/sentinel/sentinel.cpp
+++ b/configd/src/apps/sentinel/sentinel.cpp
@@ -4,6 +4,7 @@
 #include <csignal>
 #include <unistd.h>
 #include <sys/time.h>
+#include <vespa/vespalib/util/signalhandler.h>
 #include <vespa/defaults.h>
 #include "config-handler.h"
 
@@ -14,13 +15,11 @@ using namespace config;
 
 constexpr uint64_t CONFIG_TIMEOUT_MS = 3 * 60 * 1000;
 
-static int sigPermanent(int sig, void(*handler)(int));
-
-static void gracefulShutdown(int sig);
-static void sigchldHandler(int sig);
-
-sig_atomic_t stop = 0;
-static sig_atomic_t pendingWait = 0;
+static bool stop()
+{
+    return (vespalib::SignalHandler::INT.check() ||
+            vespalib::SignalHandler::TERM.check());
+}
 
 int
 main(int argc, char **argv)
@@ -49,10 +48,11 @@ main(int argc, char **argv)
 
     EV_STARTED("config-sentinel");
 
-    sigPermanent(SIGPIPE, SIG_IGN);
-    sigPermanent(SIGTERM, gracefulShutdown);
-    sigPermanent(SIGINT, gracefulShutdown);
-    sigPermanent(SIGCHLD, sigchldHandler);
+    vespalib::SignalHandler::PIPE.ignore();
+    vespalib::SignalHandler::TERM.hook();
+    vespalib::SignalHandler::INT.hook();
+    vespalib::SignalHandler::CHLD.hook();
+
     if (setenv("LC_ALL", "C", 1) != 0) {
         LOG(error, "Unable to set locale");
         exit(EXIT_FAILURE);
@@ -80,15 +80,15 @@ main(int argc, char **argv)
 
     struct timeval lastTv;
     gettimeofday(&lastTv, nullptr);
-    while (!stop) {
+    while (!stop()) {
         try {
-            pendingWait = 0;
+            vespalib::SignalHandler::CHLD.clear();
             handler.doWork();       // Check for child procs & commands
         } catch (InvalidConfigException& ex) {
             LOG(warning, "Configuration problem: (ignoring): %s",
                 ex.what());
         }
-        if (!pendingWait) {
+        if (!vespalib::SignalHandler::CHLD.check()) {
             int maxNum = 0;
             fd_set fds;
             FD_ZERO(&fds);
@@ -98,7 +98,7 @@ main(int argc, char **argv)
             tv.tv_sec = 1;
             tv.tv_usec = 0;
 
-            if (!pendingWait) {
+            if (!vespalib::SignalHandler::CHLD.check()) {
                 select(maxNum, &fds, nullptr, nullptr, &tv);
             }
         }
@@ -118,29 +118,3 @@ main(int argc, char **argv)
     EV_STOPPING("config-sentinel", "normal exit");
     return rv;
 }
-
-static void
-gracefulShutdown(int sig)
-{
-    (void)sig;
-    stop = 1;
-}
-
-static void
-sigchldHandler(int sig)
-{
-    (void)sig;
-    pendingWait = 1;
-}
-
-static int
-sigPermanent(int sig, void(*handler)(int))
-{
-    struct sigaction sa;
-
-    memset(&sa, 0, sizeof(sa));
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = 0; // no SA_RESTART!
-    sa.sa_handler = handler;
-    return sigaction(sig, &sa, nullptr);
-}
diff --git a/configd/src/apps/sentinel/service.cpp b/configd/src/apps/sentinel/service.cpp
index fc1f768f989..2f13a05eb4f 100644
--- a/configd/src/apps/sentinel/service.cpp
+++ b/configd/src/apps/sentinel/service.cpp
@@ -3,6 +3,7 @@
 #include "service.h"
 #include "output-connection.h"
 #include <vespa/vespalib/util/stringfmt.h>
+#include <vespa/vespalib/util/signalhandler.h>
 
 #include <csignal>
 #include <unistd.h>
@@ -13,7 +14,11 @@
 LOG_SETUP(".service");
 #include <vespa/log/llparser.h>
 
-extern sig_atomic_t stop;
+static bool stop()
+{
+    return (vespalib::SignalHandler::INT.check() ||
+            vespalib::SignalHandler::TERM.check());
+}
 
 using vespalib::make_string;
 
@@ -212,7 +217,7 @@ Service::start()
             static_cast<int>(getpid()));
         signal(SIGTERM, SIG_DFL);
         signal(SIGINT, SIG_DFL);
-        if (stop) {
+        if (stop()) {
             kill(getpid(), SIGTERM);
         }
         if (_restartPenalty > 0) {
@@ -315,7 +320,7 @@ Service::youExited(int status)
     } else if (_state == KILLING) {
         setState(KILLED);
     }
-    if (_isAutomatic && _config->autorestart && !stop) {
+    if (_isAutomatic && _config->autorestart && !stop()) {
         // ### Implement some rate limiting here maybe?
         LOG(debug, "%s: Has autorestart flag, restarting.", name().c_str());
         setState(READY);
diff --git a/logd/src/apps/logd/main.cpp b/logd/src/apps/logd/main.cpp
index caeaf83c7cc..00654060107 100644
--- a/logd/src/apps/logd/main.cpp
+++ b/logd/src/apps/logd/main.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <logd/errhandle.h>
-#include <logd/sigterm.h>
 #include <logd/service.h>
 #include <logd/forward.h>
 #include <logd/conf.h>
@@ -9,6 +8,7 @@
 #include <vespa/config/common/exceptions.h>
 #include <csignal>
 #include <unistd.h>
+#include <vespa/vespalib/util/sig_catch.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP("logdemon");
@@ -24,7 +24,7 @@ int main(int, char**)
 
     EV_STARTED("logdemon");
 
-    hook_signals();
+    vespalib::SigCatch catcher;
 
     const char *cfid = getenv("VESPA_CONFIG_ID");
 
@@ -50,7 +50,7 @@ int main(int, char**)
                 LOG(debug, "connection exception: %s", ex.what());
                 subscriber.closeConn();
             }
-            if (gotSignaled()) {
+            if (catcher.receivedStopSignal()) {
                 throw SigTermException("caught signal");
             }
             if (sleepcount < 60) {
@@ -60,10 +60,10 @@ int main(int, char**)
             }
             LOG(debug, "sleep %d...", sleepcount);
             for (int i = 0; i < sleepcount; i++) {
-                    sleep(1);
-                    if (gotSignaled()) {
-                        throw SigTermException("caught signal");
-                    }
+                sleep(1);
+                if (catcher.receivedStopSignal()) {
+                    throw SigTermException("caught signal");
+                }
             }
         }
     } catch (config::ConfigRuntimeException & ex) {
@@ -75,15 +75,8 @@ int main(int, char**)
         EV_STOPPING("logdemon", "bad config");
         return 1;
     } catch (SigTermException& ex) {
-        if (gotSignalNumber() == SIGTERM) {
-            LOG(debug, "stopping on SIGTERM");
-            EV_STOPPING("logdemon", "done ok.");
-        } else {
-            LOG(warning, "stopping on signal %d", gotSignalNumber());
-            char buf[100];
-            snprintf(buf, sizeof buf, "got signal %d", gotSignalNumber());
-            EV_STOPPING("logdemon", buf);
-        }
+        LOG(debug, "stopping on SIGTERM");
+        EV_STOPPING("logdemon", "done ok.");
         return 0;
     } catch (MsgException& ex) {
         LOG(error, "stopping on error: %s", ex.what());
diff --git a/logd/src/logd/CMakeLists.txt b/logd/src/logd/CMakeLists.txt
index be3fe2d1794..b436ef52876 100644
--- a/logd/src/logd/CMakeLists.txt
+++ b/logd/src/logd/CMakeLists.txt
@@ -8,7 +8,6 @@ vespa_add_library(logd STATIC
     service.cpp
     cmdbuf.cpp
     perform.cpp
-    sigterm.cpp
     DEPENDS
 )
 vespa_generate_config(logd ../main/resources/configdefinitions/logd.def)
diff --git a/logd/src/logd/sigterm.cpp b/logd/src/logd/sigterm.cpp
deleted file mode 100644
index 8fe7665865f..00000000000
--- a/logd/src/logd/sigterm.cpp
+++ /dev/null
@@ -1,27 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include "sigterm.h"
-#include <vespa/vespalib/util/signalhandler.h>
-
-void hook_signals()
-{
-    vespalib::SignalHandler::INT.hook();
-    vespalib::SignalHandler::TERM.hook();
-    vespalib::SignalHandler::PIPE.ignore();
-}
-
-bool gotSignaled()
-{
-    return (vespalib::SignalHandler::INT.check() ||
-            vespalib::SignalHandler::TERM.check());
-}
-
-int gotSignalNumber()
-{
-    if (vespalib::SignalHandler::TERM.check()) {
-        return SIGTERM;
-    }
-    if (vespalib::SignalHandler::INT.check()) {
-        return SIGINT;
-    }
-    return 0;
-}
diff --git a/logd/src/logd/sigterm.h b/logd/src/logd/sigterm.h
deleted file mode 100644
index 79b0eb7b781..00000000000
--- a/logd/src/logd/sigterm.h
+++ /dev/null
@@ -1,6 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#pragma once
-// setup for catching signals
-extern void hook_signals();
-extern bool gotSignaled();
-extern int gotSignalNumber();
diff --git a/logd/src/logd/watch.cpp b/logd/src/logd/watch.cpp
index 0558d0b4548..ad866938ab6 100644
--- a/logd/src/logd/watch.cpp
+++ b/logd/src/logd/watch.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "errhandle.h"
-#include "sigterm.h"
 #include "service.h"
 #include "forward.h"
 #include "conf.h"
@@ -13,6 +12,7 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/time.h>
+#include <vespa/vespalib/util/sig_catch.h>
 
 LOG_SETUP("");
 
@@ -196,6 +196,7 @@ Watcher::watchfile()
 
     _forwarder.sendMode();
 
+    vespalib::SigCatch catcher;
     int sleepcount = 0;
     time_t created = 0;
 
@@ -342,11 +343,11 @@ Watcher::watchfile()
             }
         }
 
-        if (gotSignaled()) {
+        if (catcher.receivedStopSignal()) {
             throw SigTermException("caught signal");
         }
         snooze(tickStart);
-        if (gotSignaled()) {
+        if (catcher.receivedStopSignal()) {
             throw SigTermException("caught signal");
         }
         if (++sleepcount > 99) {
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/.gitignore b/logforwarder/src/apps/vespa-logforwarder-start/.gitignore
index 64d78991a4c..bfa5aca99b1 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/.gitignore
+++ b/logforwarder/src/apps/vespa-logforwarder-start/.gitignore
@@ -1,3 +1 @@
 vespa-logforwarder-start
-config-logforwarder.cpp
-config-logforwarder.h
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
index f26edd8eee8..9a402bb58da 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
+++ b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
@@ -3,10 +3,10 @@ vespa_add_executable(logforwarder-start_app
     SOURCES
     main.cpp
     cf-handler.cpp
-    sig-catch.cpp
     OUTPUT_NAME vespa-logforwarder-start
     INSTALL bin
     DEPENDS
     config_cloudconfig
     configdefinitions
+    vespalib
 )
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/main.cpp b/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
index e06d3dd6d8d..8fc74fcac8e 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
+++ b/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
@@ -7,14 +7,14 @@
 LOG_SETUP("vespa-logforwarder-start");
 
 #include "cf-handler.h"
-#include "sig-catch.h"
+#include <vespa/vespalib/util/sig_catch.h>
 
 class Wrapper {
     const char *_configId;
 public:
     Wrapper(const char *cfid) : _configId(cfid) {}
     void run() {
-        SigCatch catcher;
+        vespalib::SigCatch catcher;
         CfHandler handler;
         handler.start(_configId);
         while (! catcher.receivedStopSignal()) {
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.cpp b/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.cpp
deleted file mode 100644
index b10275d515e..00000000000
--- a/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.cpp
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#include "sig-catch.h"
-#include <csignal>
-#include <unistd.h>
-#include <string.h>
-
-static int sigPermanent(int sig, void(*handler)(int));
-static void setStopFlag(int sig);
-sig_atomic_t stop = 0;
-
-SigCatch::SigCatch()
-{
-    sigPermanent(SIGPIPE, SIG_IGN);
-    sigPermanent(SIGTERM, setStopFlag);
-    sigPermanent(SIGINT, setStopFlag);
-}
-
-bool
-SigCatch::receivedStopSignal() {
-    return stop != 0;
-}
-
-static void
-setStopFlag(int sig)
-{
-    (void)sig;
-    stop = 1;
-}
-
-static int
-sigPermanent(int sig, void(*handler)(int))
-{
-    struct sigaction sa;
-
-    memset(&sa, 0, sizeof(sa));
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = 0; // no SA_RESTART!
-    sa.sa_handler = handler;
-    return sigaction(sig, &sa, nullptr);
-}
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.h b/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.h
deleted file mode 100644
index 905f37103ec..00000000000
--- a/logforwarder/src/apps/vespa-logforwarder-start/sig-catch.h
+++ /dev/null
@@ -1,8 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-class SigCatch
-{
-public:
-    SigCatch();
-    bool receivedStopSignal();
-};
diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
index 6d08c3b1126..58739ee4df6 100644
--- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt
+++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt
@@ -37,6 +37,7 @@ vespa_add_library(vespalib_vespalib_util OBJECT
     rwlock.cpp
     sequence.cpp
     sha1.cpp
+    sig_catch.cpp
     signalhandler.cpp
     simple_thread_bundle.cpp
     slaveproc.cpp
diff --git a/vespalib/src/vespa/vespalib/util/sig_catch.cpp b/vespalib/src/vespa/vespalib/util/sig_catch.cpp
new file mode 100644
index 00000000000..48bf8fbb05d
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/util/sig_catch.cpp
@@ -0,0 +1,22 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "sig_catch.h"
+#include "signalhandler.h"
+
+namespace vespalib {
+
+SigCatch::SigCatch()
+{
+    SignalHandler::PIPE.ignore();
+    SignalHandler::INT.hook();
+    SignalHandler::TERM.hook();
+}
+
+bool
+SigCatch::receivedStopSignal()
+{
+    return (SignalHandler::INT.check() ||
+            SignalHandler::TERM.check());
+}
+
+} // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/util/sig_catch.h b/vespalib/src/vespa/vespalib/util/sig_catch.h
new file mode 100644
index 00000000000..96c74ee4d07
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/util/sig_catch.h
@@ -0,0 +1,25 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+namespace vespalib {
+
+/**
+ * @brief Use this class for simple common-case signal handling.
+ **/
+
+class SigCatch
+{
+public:
+    /**
+     * Constructor installs signal handlers.
+     **/
+    SigCatch();
+
+    /**
+     * Check if a signal to stop has been received.
+     **/
+    bool receivedStopSignal();
+};
+
+} // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/util/signalhandler.cpp b/vespalib/src/vespa/vespalib/util/signalhandler.cpp
index 6bc7fcd17df..21543ef10d8 100644
--- a/vespalib/src/vespa/vespalib/util/signalhandler.cpp
+++ b/vespalib/src/vespa/vespalib/util/signalhandler.cpp
@@ -25,6 +25,7 @@ Shutdown shutdown;
 SignalHandler SignalHandler::HUP(SIGHUP);
 SignalHandler SignalHandler::INT(SIGINT);
 SignalHandler SignalHandler::TERM(SIGTERM);
+SignalHandler SignalHandler::CHLD(SIGCHLD);
 SignalHandler SignalHandler::PIPE(SIGPIPE);
 SignalHandler SignalHandler::SEGV(SIGSEGV);
 SignalHandler SignalHandler::ABRT(SIGABRT);
diff --git a/vespalib/src/vespa/vespalib/util/signalhandler.h b/vespalib/src/vespa/vespalib/util/signalhandler.h
index 6b233b2e690..f34ddba5530 100644
--- a/vespalib/src/vespa/vespalib/util/signalhandler.h
+++ b/vespalib/src/vespa/vespalib/util/signalhandler.h
@@ -67,6 +67,7 @@ public:
     static SignalHandler HUP;
     static SignalHandler INT;
     static SignalHandler TERM;
+    static SignalHandler CHLD;
     static SignalHandler PIPE;
     static SignalHandler SEGV;
     static SignalHandler ABRT;