mgorny updated this revision to Diff 338170.
mgorny marked an inline comment as done.
mgorny added a comment.
Fixed the iterator, formatting and added a test for adding, running and
removing multiple handlers.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100418/new/
https://reviews.llvm.org/D100418
Files:
lldb/include/lldb/Host/MainLoop.h
lldb/source/Host/common/MainLoop.cpp
lldb/unittests/Host/MainLoopTest.cpp
Index: lldb/unittests/Host/MainLoopTest.cpp
===================================================================
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -152,4 +152,49 @@
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
Index: lldb/source/Host/common/MainLoop.cpp
===================================================================
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -302,13 +302,15 @@
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 @@
&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 @@
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 @@
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: lldb/include/lldb/Host/MainLoop.h
===================================================================
--- lldb/include/lldb/Host/MainLoop.h
+++ 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 @@
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 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 @@
};
struct SignalInfo {
- Callback callback;
+ std::list<Callback> callbacks;
#if HAVE_SIGACTION
struct sigaction old_action;
#endif
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
