[lldb] [MainLoop] Support multiple callbacks per signal
authorMichał Górny <mgorny@moritz.systems>
Tue, 13 Apr 2021 22:54:08 +0000 (00:54 +0200)
committerMichał Górny <mgorny@moritz.systems>
Wed, 21 Apr 2021 10:18:20 +0000 (12:18 +0200)
Support registering multiple callbacks for a single signal.  This is
necessary to support multiple co-existing native process instances, with
separate SIGCHLD handlers.

The system signal handler is registered on first request, additional
callback are added on subsequent requests.  The system signal handler
is removed when last callback is unregistered.

Differential Revision: https://reviews.llvm.org/D100418

lldb/include/lldb/Host/MainLoop.h
lldb/source/Host/common/MainLoop.cpp
lldb/unittests/Host/MainLoopTest.cpp

index 9ca5040..06785bb 100644 (file)
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "llvm/ADT/DenseMap.h"
 #include <csignal>
+#include <list>
 
 #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
 #define SIGNAL_POLLING_UNSUPPORTED 1
@@ -68,7 +69,7 @@ public:
 protected:
   void UnregisterReadObject(IOObject::WaitableHandle handle) override;
 
-  void UnregisterSignal(int signo);
+  void UnregisterSignal(int signo, std::list<Callback>::iterator callback_it);
 
 private:
   void ProcessReadObject(IOObject::WaitableHandle handle);
@@ -76,14 +77,16 @@ private:
 
   class SignalHandle {
   public:
-    ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo); }
+    ~SignalHandle() { m_mainloop.UnregisterSignal(m_signo, m_callback_it); }
 
   private:
-    SignalHandle(MainLoop &mainloop, int signo)
-        : m_mainloop(mainloop), m_signo(signo) {}
+    SignalHandle(MainLoop &mainloop, int signo,
+                 std::list<Callback>::iterator callback_it)
+        : m_mainloop(mainloop), m_signo(signo), m_callback_it(callback_it) {}
 
     MainLoop &m_mainloop;
     int m_signo;
+    std::list<Callback>::iterator m_callback_it;
 
     friend class MainLoop;
     SignalHandle(const SignalHandle &) = delete;
@@ -91,7 +94,7 @@ private:
   };
 
   struct SignalInfo {
-    Callback callback;
+    std::list<Callback> callbacks;
 #if HAVE_SIGACTION
     struct sigaction old_action;
 #endif
index 02cabbc..cdcb052 100644 (file)
@@ -302,13 +302,15 @@ MainLoop::RegisterSignal(int signo, const Callback &callback, Status &error) {
   error.SetErrorString("Signal polling is not supported on this platform.");
   return nullptr;
 #else
-  if (m_signals.find(signo) != m_signals.end()) {
-    error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
-    return nullptr;
+  auto signal_it = m_signals.find(signo);
+  if (signal_it != m_signals.end()) {
+    auto callback_it = signal_it->second.callbacks.insert(
+        signal_it->second.callbacks.end(), callback);
+    return SignalHandleUP(new SignalHandle(*this, signo, callback_it));
   }
 
   SignalInfo info;
-  info.callback = callback;
+  info.callbacks.push_back(callback);
   struct sigaction new_action;
   new_action.sa_sigaction = &SignalHandler;
   new_action.sa_flags = SA_SIGINFO;
@@ -338,9 +340,10 @@ MainLoop::RegisterSignal(int signo, const Callback &callback, Status &error) {
                         &new_action.sa_mask, &old_set);
   assert(ret == 0 && "pthread_sigmask failed");
   info.was_blocked = sigismember(&old_set, signo);
-  m_signals.insert({signo, info});
+  auto insert_ret = m_signals.insert({signo, info});
 
-  return SignalHandleUP(new SignalHandle(*this, signo));
+  return SignalHandleUP(new SignalHandle(
+      *this, signo, insert_ret.first->second.callbacks.begin()));
 #endif
 }
 
@@ -350,13 +353,19 @@ void MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
   assert(erased);
 }
 
-void MainLoop::UnregisterSignal(int signo) {
+void MainLoop::UnregisterSignal(int signo,
+                                std::list<Callback>::iterator callback_it) {
 #if SIGNAL_POLLING_UNSUPPORTED
   Status("Signal polling is not supported on this platform.");
 #else
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
+  it->second.callbacks.erase(callback_it);
+  // Do not remove the signal handler unless all callbacks have been erased.
+  if (!it->second.callbacks.empty())
+    return;
+
   sigaction(signo, &it->second.old_action, nullptr);
 
   sigset_t set;
@@ -398,8 +407,14 @@ Status MainLoop::Run() {
 
 void MainLoop::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
-  if (it != m_signals.end())
-    it->second.callback(*this); // Do the work
+  if (it != m_signals.end()) {
+    // The callback may actually register/unregister signal handlers,
+    // so we need to create a copy first.
+    llvm::SmallVector<Callback, 4> callbacks_to_run{
+        it->second.callbacks.begin(), it->second.callbacks.end()};
+    for (auto &x : callbacks_to_run)
+      x(*this); // Do the work
+  }
 }
 
 void MainLoop::ProcessReadObject(IOObject::WaitableHandle handle) {
index c680af7..50a49b9 100644 (file)
@@ -152,4 +152,49 @@ TEST_F(MainLoopTest, UnmonitoredSignal) {
   killer.join();
   ASSERT_EQ(1u, callback_count);
 }
+
+// Test that two callbacks can be registered for the same signal
+// and unregistered independently.
+TEST_F(MainLoopTest, TwoSignalCallbacks) {
+  MainLoop loop;
+  Status error;
+  int callback2_count = 0;
+  int callback3_count = 0;
+
+  auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+  ASSERT_TRUE(error.Success());
+
+  {
+    // Run a single iteration with two callbacks enabled.
+    auto handle2 = loop.RegisterSignal(
+        SIGUSR1, [&](MainLoopBase &loop) { ++callback2_count; }, error);
+    ASSERT_TRUE(error.Success());
+
+    kill(getpid(), SIGUSR1);
+    ASSERT_TRUE(loop.Run().Success());
+    ASSERT_EQ(1u, callback_count);
+    ASSERT_EQ(1u, callback2_count);
+    ASSERT_EQ(0u, callback3_count);
+  }
+
+  {
+    // Make sure that remove + add new works.
+    auto handle3 = loop.RegisterSignal(
+        SIGUSR1, [&](MainLoopBase &loop) { ++callback3_count; }, error);
+    ASSERT_TRUE(error.Success());
+
+    kill(getpid(), SIGUSR1);
+    ASSERT_TRUE(loop.Run().Success());
+    ASSERT_EQ(2u, callback_count);
+    ASSERT_EQ(1u, callback2_count);
+    ASSERT_EQ(1u, callback3_count);
+  }
+
+  // Both extra callbacks should be unregistered now.
+  kill(getpid(), SIGUSR1);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(3u, callback_count);
+  ASSERT_EQ(1u, callback2_count);
+  ASSERT_EQ(1u, callback3_count);
+}
 #endif