Handle signals locally in socket manager main loop

* replace SignalService with a local descriptor
* handle the descriptor directly in the main loop
* drop the now unused m_working and MainLoopStop()

White at it, also drop the harmful TEMP_FAILURE_RETRY when calling
close() on service sockets.

Change-Id: I172456d1762aaed4c4f0dd46a49732aa28d9c5d6

diff --git a/src/server/main/include/socket-manager.h b/src/server/main/include/socket-manager.h
index e8113d080c966c1354895e3174952b7462dc9a4b..9737382fc7ba32ebe6059b6ae69aaef012013b9a 100644 (file)
--- a/src/server/main/include/socket-manager.h
+++ b/src/server/main/include/socket-manager.h
@@ -50,7 +50,6 @@ public:
     SocketManager();
     virtual ~SocketManager();
     virtual void MainLoop();
-    virtual void MainLoopStop();
 
     virtual void RegisterSocketService(GenericSocketService *service);
     virtual void Close(ConnectionID connectionID);
@@ -68,6 +67,7 @@ protected:
     void ReadyForRead(int sock);
     void ReadyForWrite(int sock);
     void ReadyForAccept(int sock);
+    bool GotSigTerm() const;
     void ProcessQueue(void);
     void NotifyMe(void);
     void CloseSocket(int sock);
@@ -116,9 +116,8 @@ protected:
     fd_set m_readSet;
     fd_set m_writeSet;
     int m_maxDesc = 0;
-    int m_notifyMe;
+    int m_signalFd, m_notifyMe;
     int m_counter = 0;
-    bool m_working = false;
     std::mutex m_eventQueueMutex;
     std::queue<WriteBuffer> m_writeBufferQueue;
     std::queue<ConnectionID> m_closeQueue;

diff --git a/src/server/main/socket-manager.cpp b/src/server/main/socket-manager.cpp
index 593f0e4d199c15a83d670fe7f71617da92c6378e..fd945d5c0b227b65c1702ce2e456c28a95b0d87a 100644 (file)
--- a/src/server/main/socket-manager.cpp
+++ b/src/server/main/socket-manager.cpp
@@ -63,53 +63,6 @@ const time_t SOCKET_TIMEOUT = 300;
 
 namespace SecurityManager {
 
-struct SignalService : public GenericSocketService {
-    int GetDescriptor() {
-        LogInfo("set up");
-        sigset_t mask;
-        sigemptyset(&mask);
-        sigaddset(&mask, SIGTERM);
-        sigaddset(&mask, SIGCHLD);
-        if (-1 == pthread_sigmask(SIG_BLOCK, &mask, NULL))
-            return -1;
-        return signalfd(-1, &mask, 0);
-    }
-
-    ServiceDescriptionVector GetServiceDescription() {
-        return ServiceDescriptionVector();
-    }
-
-    void Event(AcceptEvent &&) { } // not supported
-    void Event(WriteEvent &&) { }  // not supported
-    void Event(CloseEvent &&) { }  // not supported
-
-    void Event(ReadEvent &&event) {
-        LogDebug("Get signal information");
-
-        if (sizeof(struct signalfd_siginfo) != event.rawBuffer.size()) {
-            LogError("Wrong size of signalfd_siginfo struct. Expected: "
-                << sizeof(signalfd_siginfo) << " Get: "
-                << event.rawBuffer.size());
-            return;
-        }
-
-        signalfd_siginfo *siginfo = (signalfd_siginfo*)(&(event.rawBuffer[0]));
-
-        if (siginfo->ssi_signo == SIGTERM) {
-            LogInfo("Got signal: SIGTERM");
-            auto manager = dynamic_cast<SocketManager*>(m_serviceManager);
-            if (manager)
-                manager->MainLoopStop();
-            return;
-        } else if (siginfo->ssi_signo == SIGCHLD) {
-            (void)waitpid(-1, nullptr, WNOHANG);
-            return;
-        }
-
-        LogInfo("This should not happend. Got signal: " << siginfo->ssi_signo);
-    }
-};
-
 void SocketManager::RegisterFdForReading(int fd) {
     FD_SET(fd, &m_readSet);
     m_maxDesc = std::max(m_maxDesc, fd);
@@ -148,25 +101,28 @@ SocketManager::SocketManager()
 {
     FD_ZERO(&m_readSet);
     FD_ZERO(&m_writeSet);
+
+    // std::thread bases on pthread so this should work fine
+    sigset_t set;
+    sigemptyset(&set);
+    sigaddset(&set, SIGTERM);
+    sigaddset(&set, SIGCHLD);
+    if (auto err = pthread_sigmask(SIG_BLOCK, &set, nullptr))
+        ThrowMsg(Exception::InitFailed, "Error in pthread_sigmask: " << err);
+
+    // add support for TERM signal (passed from systemd)
+    if ((m_signalFd = signalfd(-1, &set, 0)) < 0) {
+        int err = errno;
+        ThrowMsg(Exception::InitFailed, "Error in signalfd: " << GetErrnoString(err));
+    }
+    RegisterFdForReading(m_signalFd);
+
     if ((m_notifyMe = eventfd(0, 0)) < 0) {
         int err = errno;
         ThrowMsg(Exception::InitFailed, "Error in eventfd: " << GetErrnoString(err));
     }
     LogInfo("Eventfd desc: " << m_notifyMe);
     RegisterFdForReading(m_notifyMe);
-
-    // add support for TERM signal (passed from systemd)
-    auto *signalService = new SignalService;
-    signalService->SetSocketManager(this);
-    int filefd = signalService->GetDescriptor();
-    if (-1 == filefd) {
-        LogError("Error in SignalService.GetDescriptor()");
-        delete signalService;
-    } else {
-        auto &desc2 = CreateDefaultReadSocketDescription(filefd, false);
-        desc2.service = signalService;
-        LogInfo("SignalService mounted on " << filefd << " descriptor");
-    }
 }
 
 SocketManager::~SocketManager() {
@@ -190,6 +146,7 @@ SocketManager::~SocketManager() {
             close(i);
 
     // All service sockets have been closed. Close internal descriptors.
+    close(m_signalFd);
     close(m_notifyMe);
 }
 
@@ -215,6 +172,31 @@ void SocketManager::ReadyForAccept(int sock) {
     desc.service->Event(std::move(event));
 }
 
+// true if quit mainloop
+bool SocketManager::GotSigTerm() const {
+    LogDebug("Get signal information");
+
+    struct signalfd_siginfo info;
+    const auto s = TEMP_FAILURE_RETRY(read(m_signalFd, &info, sizeof info));
+    if (s != sizeof info) {
+        LogError("Wrong signalfd read size. Expected: " << sizeof info << " Got: " << s);
+        return false;
+    }
+
+    switch (info.ssi_signo) {
+        case SIGTERM:
+            LogInfo("Got signal: SIGTERM");
+            return true;
+        case SIGCHLD:
+            (void)waitpid(-1, nullptr, WNOHANG);
+            break;
+        default:
+            LogError("This should not happen. Got signal: " << info.ssi_signo);
+            break;
+    }
+    return false;
+}
+
 void SocketManager::ReadyForRead(int sock) {
     if (m_socketDescriptionVector[sock].isListen) {
         ReadyForAccept(sock);
@@ -292,7 +274,6 @@ void SocketManager::MainLoop() {
     // Daemon is ready to work.
     sd_notify(0, "READY=1");
 
-    m_working = true;
     for (;;) {
         fd_set readSet = m_readSet;
         fd_set writeSet = m_writeSet;
@@ -383,9 +364,12 @@ void SocketManager::MainLoop() {
             continue;
         }
 
-        if (FD_ISSET(m_notifyMe, &readSet)) {
-            if (!m_working)
+        if (FD_ISSET(m_signalFd, &readSet)) {
+            if (GotSigTerm())
                 return;
+            FD_CLR(m_signalFd, &readSet);
+        }
+        if (FD_ISSET(m_notifyMe, &readSet)) {
             eventfd_t dummyValue;
             TEMP_FAILURE_RETRY(eventfd_read(m_notifyMe, &dummyValue));
             FD_CLR(m_notifyMe, &readSet);
@@ -405,12 +389,6 @@ void SocketManager::MainLoop() {
     }
 }
 
-void SocketManager::MainLoopStop()
-{
-    m_working = false;
-    NotifyMe();
-}
-
 int SocketManager::GetSocketFromSystemD(
     const GenericSocketService::ServiceDescription &desc)
 {
@@ -652,7 +630,7 @@ void SocketManager::CloseSocket(int sock) {
     else
         LogError("Critical! Service is NULL! This should never happend!");
 
-    TEMP_FAILURE_RETRY(close(sock));
+    close(sock);
     FD_CLR(sock, &m_readSet);
     FD_CLR(sock, &m_writeSet);
     LogDebug("Closing socket: " << sock << "  finished..");