[Lldb-commits] [lldb] 08ce2ba - [lldb] [MainLoop] Support multiple callbacks per signal

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 21 03:18:29 PDT 2021


Author: Michał Górny
Date: 2021-04-21T12:18:20+02:00
New Revision: 08ce2ba518031425643ce3a4a0476f770c9b8dcd

URL: https://github.com/llvm/llvm-project/commit/08ce2ba518031425643ce3a4a0476f770c9b8dcd
DIFF: https://github.com/llvm/llvm-project/commit/08ce2ba518031425643ce3a4a0476f770c9b8dcd.diff

LOG: [lldb] [MainLoop] Support multiple callbacks per signal

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Host/MainLoop.h b/lldb/include/lldb/Host/MainLoop.h
index 9ca5040b60a89..06785bbdbe246 100644
--- a/lldb/include/lldb/Host/MainLoop.h
+++ b/lldb/include/lldb/Host/MainLoop.h
@@ -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 @@ class MainLoop : public MainLoopBase {
 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 @@ class MainLoop : public MainLoopBase {
 
   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 @@ class MainLoop : public MainLoopBase {
   };
 
   struct SignalInfo {
-    Callback callback;
+    std::list<Callback> callbacks;
 #if HAVE_SIGACTION
     struct sigaction old_action;
 #endif

diff  --git a/lldb/source/Host/common/MainLoop.cpp b/lldb/source/Host/common/MainLoop.cpp
index 02cabbc93550a..cdcb052217960 100644
--- a/lldb/source/Host/common/MainLoop.cpp
+++ b/lldb/source/Host/common/MainLoop.cpp
@@ -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) {

diff  --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp
index c680af70d1bd1..50a49b92d48bf 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -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


        


More information about the lldb-commits mailing list