From 5bafef3957bc32a2deb5a917c7773fcd92e8b953 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Fri, 12 Apr 2013 12:24:34 +0200
Subject: engine: support additional signals in SuspendFlags

It can be useful to use additional signals like SIGURG or SIGIO
(typically not used elsewhere) for simple interprocess
communication. SuspendFlags now supports that by letting the process
catch more than just SIGINT and SIGTERM and recording which signals
were received.

activate() calls can happen when already active, for example when
syncevo-dbus-helper instantiates SyncContext, which activates signal
handling for the duration of the sync. This was not handled well
before, leaking previously allocated FDs and not restoring signal
handlers correctly. Because the process quit anyway soon, this did not
matter in practice.

Now the code explicitly checks for this and returns the existing Guard
in all following activate() calls, without changing the signal
handling.
---
 src/syncevo/SuspendFlags.cpp | 79 +++++++++++++++++++++++++++++---------------
 src/syncevo/SuspendFlags.h   | 42 +++++++++++++++++++----
 2 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/src/syncevo/SuspendFlags.cpp b/src/syncevo/SuspendFlags.cpp
index b47a8524..0f46bc49 100644
--- a/src/syncevo/SuspendFlags.cpp
+++ b/src/syncevo/SuspendFlags.cpp
@@ -36,9 +36,11 @@ SE_BEGIN_CXX
 SuspendFlags::SuspendFlags() :
     m_level(Logger::INFO),
     m_state(NORMAL),
+    m_receivedSignals(0),
     m_lastSuspend(0),
     m_senderFD(-1),
-    m_receiverFD(-1)
+    m_receiverFD(-1),
+    m_activeSignals(0)
 {
 }
 
@@ -148,10 +150,14 @@ boost::shared_ptr<SuspendFlags::StateBlocker> SuspendFlags::block(boost::weak_pt
     return res;
 }
 
-boost::shared_ptr<SuspendFlags::Guard> SuspendFlags::activate()
+boost::shared_ptr<SuspendFlags::Guard> SuspendFlags::activate(uint32_t sigmask)
 {
     SE_LOG_DEBUG(NULL, "SuspendFlags: (re)activating, currently %s",
                  m_senderFD > 0 ? "active" : "inactive");
+    if (m_senderFD > 0) {
+        return m_guard.lock();
+    }
+
     int fds[2];
     if (pipe(fds)) {
         SE_THROW(StringPrintf("allocating pipe for signals failed: %s", strerror(errno)));
@@ -163,8 +169,11 @@ boost::shared_ptr<SuspendFlags::Guard> SuspendFlags::activate()
     m_receiverFD = fds[0];
     SE_LOG_DEBUG(NULL, "SuspendFlags: activating signal handler(s) with fds %d->%d",
                  m_senderFD, m_receiverFD);
-    sigaction(SIGINT, NULL, &m_oldSigInt);
-    sigaction(SIGTERM, NULL, &m_oldSigTerm);
+    for (int sig = 0; sig < 32; sig++) {
+        if (sigmask & (1<<sig)) {
+            sigaction(sig, NULL, m_oldSignalHandlers + sig);
+        }
+    }
 
     struct sigaction new_action;
     memset(&new_action, 0, sizeof(new_action));
@@ -173,22 +182,27 @@ boost::shared_ptr<SuspendFlags::Guard> SuspendFlags::activate()
     // don't let processing of SIGINT be interrupted
     // of SIGTERM and vice versa, if we are doing the
     // handling
-    if (m_oldSigInt.sa_handler == SIG_DFL) {
-        sigaddset(&new_action.sa_mask, SIGINT);
-    }
-    if (m_oldSigTerm.sa_handler == SIG_DFL) {
-        sigaddset(&new_action.sa_mask, SIGTERM);
-    }
-    if (m_oldSigInt.sa_handler == SIG_DFL) {
-        sigaction(SIGINT, &new_action, NULL);
-        SE_LOG_DEBUG(NULL, "SuspendFlags: catch SIGINT");
+    for (int sig = 0; sig < 32; sig++) {
+        if (sigmask & (1<<sig)) {
+            if (m_oldSignalHandlers[sig].sa_handler == SIG_DFL) {
+                sigaddset(&new_action.sa_mask, sig);
+            }
+        }
     }
-    if (m_oldSigTerm.sa_handler == SIG_DFL) {
-        sigaction(SIGTERM, &new_action, NULL);
-        SE_LOG_DEBUG(NULL, "SuspendFlags: catch SIGTERM");
+
+    for (int sig = 0; sig < 32; sig++) {
+        if (sigmask & (1<<sig)) {
+            if (m_oldSignalHandlers[sig].sa_handler == SIG_DFL) {
+                sigaction(sig, &new_action, NULL);
+                SE_LOG_DEBUG(NULL, "SuspendFlags: catch signal %d", sig);
+            }
+        }
     }
+    m_activeSignals = sigmask;
+    boost::shared_ptr<Guard> guard(new GLibGuard(m_receiverFD));
+    m_guard = guard;
 
-    return boost::shared_ptr<Guard>(new GLibGuard(m_receiverFD));
+    return guard;
 }
 
 void SuspendFlags::deactivate()
@@ -196,14 +210,19 @@ void SuspendFlags::deactivate()
     SE_LOG_DEBUG(NULL, "SuspendFlags: deactivating fds %d->%d",
                  m_senderFD, m_receiverFD);
     if (m_receiverFD >= 0) {
-        sigaction(SIGTERM, &m_oldSigTerm, NULL);
-        sigaction(SIGINT, &m_oldSigInt, NULL);
+        for (int sig = 0; sig < 32; sig++) {
+            if (m_activeSignals & (1<<sig)) {
+                sigaction(sig, m_oldSignalHandlers + sig, NULL);
+            }
+        }
+        m_activeSignals = 0;
         SE_LOG_DEBUG(NULL, "SuspendFlags: close m_receiverFD %d", m_receiverFD);
         close(m_receiverFD);
         SE_LOG_DEBUG(NULL, "SuspendFlags: close m_senderFD %d", m_senderFD);
         close(m_senderFD);
         m_receiverFD = -1;
         m_senderFD = -1;
+        m_guard.reset();
         SE_LOG_DEBUG(NULL, "SuspendFlags: done with deactivation");
     }
 }
@@ -245,19 +264,24 @@ void SuspendFlags::handleSignal(int sig)
                 msg = SUSPEND_AGAIN;
             }
             break;
-        case SuspendFlags::ABORT:
+        case ABORT:
             msg = ABORT_AGAIN;
             break;
-        case SuspendFlags::ABORT_AGAIN:
-        case SuspendFlags::SUSPEND_AGAIN:
+        case ABORT_AGAIN:
+        case SUSPEND_AGAIN:
+        case ABORT_MAX:
             // shouldn't happen
+            msg = ABORT_MAX;
             break;
         }
+    default:
+        msg = ABORT_MAX;
         break;
     }
     }
     if (me.m_senderFD >= 0) {
-        write(me.m_senderFD, &msg, 1);
+        unsigned char msg2[2] = { (unsigned char)(ABORT_MAX + sig), msg };
+        write(me.m_senderFD, msg2, msg == ABORT_MAX ? 1 : 2);
     }
 }
 
@@ -282,10 +306,13 @@ void SuspendFlags::printSignals()
             case ABORT_AGAIN:
                 str = "Already aborting as requested earlier ...";
                 break;
+            default: {
+                int sig = msg - ABORT_MAX;
+                SE_LOG_DEBUG(NULL, "reveived signal %d", sig);
+                m_receivedSignals |= 1<<sig;
             }
-            if (!str) {
-                SE_LOG_DEBUG(NULL, "internal error: received invalid signal msg %d", msg);
-            } else {
+            }
+            if (str) {
                 SE_LOG(NULL, m_level, "%s", str);
             }
             m_stateChanged(*this);
diff --git a/src/syncevo/SuspendFlags.h b/src/syncevo/SuspendFlags.h
index a45c1154..e6f9c07f 100644
--- a/src/syncevo/SuspendFlags.h
+++ b/src/syncevo/SuspendFlags.h
@@ -56,7 +56,9 @@ class SuspendFlags
         /** suspend sync request received again (only written to event FD, not returned by getState()) */
         SUSPEND_AGAIN,
         /** abort sync request received again (only written to event FD, not returned by getState()) */
-        ABORT_AGAIN
+        ABORT_AGAIN,
+
+        ABORT_MAX
     };
 
     /** access to singleton */
@@ -72,6 +74,12 @@ class SuspendFlags
      */
     State getState() const;
 
+    /**
+     * Returns or-ed mask of all signals handled so far.
+     * See activate().
+     */
+    uint32_t getReceivedSignals() const { return m_receivedSignals; }
+
     /**
      * Throws a "aborting as requested by user" StatusException with
      * LOCERR_USERABORT as status code if the current state is not
@@ -83,7 +91,7 @@ class SuspendFlags
     /**
      * Users of this class can read a single char for each received
      * signal from this file descriptor. The char is the State that
-     * was entered by that signal. This can be used to be notified
+      * was entered by that signal. This can be used to be notified
      * immediately about changes, without having to poll.
      *
      * -1 if not activated.
@@ -96,11 +104,25 @@ class SuspendFlags
     };
 
     /**
-     * Allocate file descriptors, set signal handlers for SIGINT and
-     * SIGTERM. Once the returned guard is freed, it will
-     * automatically deactivate signal handling.
+     * Allocate file descriptors, set signal handlers for the chosen
+     * signals (SIGINT and SIGTERM by default). Once the returned
+     * guard is freed, it will automatically deactivate signal
+     * handling.
+     *
+     * Additional signals like SIGURG or SIGIO may also be used. It is
+     * unlikely that any library used by SyncEvolution occupies these
+     * signals for its own use.
+     *
+     * Only SIGINT and SIGTERM influence the overall State. All
+     * received signals, including SIGINT and SIGTERM, are recorded
+     * and can be retrieved via getReceivedSignals().
+     *
+     * It is possible to call activate multiple times. All following
+     * calls do nothing except creating a new reference to the same
+     * guard.  In particular they cannot add or remove handled
+     * signals.
      */
-    boost::shared_ptr<Guard> activate();
+    boost::shared_ptr<Guard> activate(uint32_t sigmask = (1<<SIGINT)|(1<<SIGTERM));
 
     /**
      * Retrieve state changes pushed into pipe by signal
@@ -162,12 +184,18 @@ class SuspendFlags
     /** state as observed by signal handler */
     State m_state;
 
+    /** or-ed bit mask of all received signals */
+    uint32_t m_receivedSignals;
+
     /** time is measured inside signal handler */
     time_t m_lastSuspend;
 
     int m_senderFD, m_receiverFD;
-    struct sigaction m_oldSigInt, m_oldSigTerm;
+    // For the sake of simplicity we only support signals in the 1-31 range.
+    uint32_t m_activeSignals;
+    struct sigaction m_oldSignalHandlers[32];
 
+    boost::weak_ptr<Guard> m_guard;
     boost::weak_ptr<StateBlocker> m_suspendBlocker, m_abortBlocker;
     boost::shared_ptr<StateBlocker> block(boost::weak_ptr<StateBlocker> &blocker);
 };
-- 
cgit v1.2.3