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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits