diff options
author | Konrad Lipinski <k.lipinski2@samsung.com> | 2022-07-12 13:36:59 +0200 |
---|---|---|
committer | Konrad Lipinski <k.lipinski2@samsung.com> | 2022-07-12 14:21:54 +0200 |
commit | 788ee5f1c2ffd0c88d6ee36db913f7cb31563c21 (patch) | |
tree | 37316885c3a31a6fdba968d5f04fac1475e405e3 | |
parent | d7678d368cf2653eb29e9b54cca97d9574ea4797 (diff) | |
download | security-manager-788ee5f1c2ffd0c88d6ee36db913f7cb31563c21.tar.gz security-manager-788ee5f1c2ffd0c88d6ee36db913f7cb31563c21.tar.bz2 security-manager-788ee5f1c2ffd0c88d6ee36db913f7cb31563c21.zip |
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
-rw-r--r-- | src/server/main/include/socket-manager.h | 5 | ||||
-rw-r--r-- | src/server/main/socket-manager.cpp | 118 |
2 files changed, 50 insertions, 73 deletions
diff --git a/src/server/main/include/socket-manager.h b/src/server/main/include/socket-manager.h index e8113d08..9737382f 100644 --- 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 593f0e4d..fd945d5c 100644 --- 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.."); |