[Lldb-commits] [lldb] 8612926 - [lldb] Fix race condition in Process::WaitForProcessToStop() (#144919)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 15 09:33:03 PDT 2025
Author: athierry-oct
Date: 2025-07-15T09:33:00-07:00
New Revision: 8612926c306c5191a5fb385dd11467728c59e982
URL: https://github.com/llvm/llvm-project/commit/8612926c306c5191a5fb385dd11467728c59e982
DIFF: https://github.com/llvm/llvm-project/commit/8612926c306c5191a5fb385dd11467728c59e982.diff
LOG: [lldb] Fix race condition in Process::WaitForProcessToStop() (#144919)
This PR addresses a race condition encountered when using LLDB through
the Python scripting interface.
I'm relatively new to LLDB, so feedback is very welcome, especially if
there's a more appropriate way to address this issue.
### Bug Description
When running a script that repeatedly calls
`debugger.GetListener().WaitForEvent()` in a loop, and at some point
invokes `process.Kill()` from within that loop to terminate the session,
a race condition can occur if `process.Kill()` is called around the same
time a breakpoint is hit.
### Race Condition Details
The issue arises when the following sequence of events happens:
1. The process's **private state** transitions to `stopped` when the
breakpoint is hit.
2. `process.Kill()` calls `Process::Destroy()`, which invokes
`Process::WaitForProcessToStop()`. At this point:
- `private_state = stopped`
- `public_state = running` (the public state has not yet been updated)
3. The **public stop event** is broadcast **before** the hijack listener
is installed.
4. As a result, the stop event is delivered to the **non-hijack
listener**.
5. The interrupt request sent by `Process::StopForDestroyOrDetach()` is
ignored because the process is already stopped (`private_state =
stopped`).
6. No public stop event reaches the hijack listener.
7. `Process::WaitForProcessToStop()` hangs waiting for a public stop
event, but the event is never received.
8. `process.Kill()` times out after 20 seconds
### Fix Summary
This patch modifies `Process::WaitForProcessToStop()` to ensure that any
pending events in the non-hijack listener queue are processed before
checking the hijack listener. This guarantees that any missed public
state change events are handled, preventing the hang.
### Additional Context
A discussion of this issue, including a script to reproduce the bug, can
be found here:
[LLDB hangs when killing process at the same time a breakpoint is
hit](https://discourse.llvm.org/t/lldb-hangs-when-killing-process-at-the-same-time-a-breakpoint-is-hit)
Added:
Modified:
lldb/include/lldb/Utility/Listener.h
lldb/source/Utility/Broadcaster.cpp
lldb/source/Utility/Listener.cpp
lldb/unittests/Utility/ListenerTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/Listener.h b/lldb/include/lldb/Utility/Listener.h
index d48816ec0ea4d..393169091390c 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -53,6 +53,13 @@ class Listener : public std::enable_shared_from_this<Listener> {
void AddEvent(lldb::EventSP &event);
+ /// Transfers all events matching the specified broadcaster and type to the
+ /// destination listener. This can be useful when setting up a hijack listener,
+ /// to ensure that no relevant events are missed.
+ void MoveEvents(lldb::ListenerSP destination,
+ Broadcaster *broadcaster, // nullptr for any broadcaster
+ uint32_t event_type_mask);
+
void Clear();
const char *GetName() { return m_name.c_str(); }
diff --git a/lldb/source/Utility/Broadcaster.cpp b/lldb/source/Utility/Broadcaster.cpp
index c6b2606afe0c8..41b782fee55fa 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -335,6 +335,18 @@ bool Broadcaster::BroadcasterImpl::HijackBroadcaster(
"{0} Broadcaster(\"{1}\")::HijackBroadcaster (listener(\"{2}\")={3})",
static_cast<void *>(this), GetBroadcasterName(),
listener_sp->m_name.c_str(), static_cast<void *>(listener_sp.get()));
+
+ // Move pending events from the previous listener to the hijack listener.
+ // This ensures that no relevant event queued before the transition is missed
+ // by the hijack listener.
+ ListenerSP prev_listener;
+ if (!m_hijacking_listeners.empty())
+ prev_listener = m_hijacking_listeners.back();
+ else if (m_primary_listener_sp)
+ prev_listener = m_primary_listener_sp;
+ if (prev_listener && listener_sp)
+ prev_listener->MoveEvents(listener_sp, &m_broadcaster, event_mask);
+
m_hijacking_listeners.push_back(listener_sp);
m_hijacking_masks.push_back(event_mask);
return true;
@@ -367,6 +379,19 @@ void Broadcaster::BroadcasterImpl::RestoreBroadcaster() {
static_cast<void *>(this), GetBroadcasterName(),
listener_sp->m_name.c_str(),
static_cast<void *>(listener_sp.get()));
+
+ // Move any remaining events from the hijack listener back to
+ // the previous listener. This ensures that no events are dropped when
+ // restoring the original listener.
+ ListenerSP prev_listener;
+ if (m_hijacking_listeners.size() > 1)
+ prev_listener = m_hijacking_listeners[m_hijacking_listeners.size() - 2];
+ else if (m_primary_listener_sp)
+ prev_listener = m_primary_listener_sp;
+ if (listener_sp && prev_listener && !m_hijacking_masks.empty())
+ listener_sp->MoveEvents(prev_listener, &m_broadcaster,
+ m_hijacking_masks.back());
+
m_hijacking_listeners.pop_back();
}
if (!m_hijacking_masks.empty())
diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index d4ce3bf25ec5a..a9b733579d2f7 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -176,6 +176,33 @@ void Listener::AddEvent(EventSP &event_sp) {
m_events_condition.notify_all();
}
+void Listener::MoveEvents(
+ ListenerSP destination,
+ Broadcaster *broadcaster, // nullptr for any broadcaster
+ uint32_t event_type_mask) {
+ Log *log = GetLog(LLDBLog::Events);
+
+ std::lock_guard<std::mutex> guard(m_events_mutex);
+ auto pos = m_events.begin();
+ while (pos != m_events.end()) {
+ EventSP &event_sp = *pos;
+ if (event_sp &&
+ ((broadcaster == nullptr) || event_sp->BroadcasterIs(broadcaster)) &&
+ (event_type_mask == 0 || event_type_mask & event_sp->GetType())) {
+ LLDB_LOGF(
+ log, "%p Listener('%s')::MoveEvents moving event %p to %p('%s')",
+ static_cast<void *>(this), m_name.c_str(),
+ static_cast<void *>(event_sp.get()),
+ static_cast<void *>(destination.get()), destination->GetName());
+
+ destination->AddEvent(event_sp);
+ pos = m_events.erase(pos);
+ } else {
+ ++pos;
+ }
+ }
+}
+
bool Listener::FindNextEventInternal(
std::unique_lock<std::mutex> &lock,
Broadcaster *broadcaster, // nullptr for any broadcaster
diff --git a/lldb/unittests/Utility/ListenerTest.cpp b/lldb/unittests/Utility/ListenerTest.cpp
index f7aa0f59d1848..a980a9f01c488 100644
--- a/lldb/unittests/Utility/ListenerTest.cpp
+++ b/lldb/unittests/Utility/ListenerTest.cpp
@@ -150,3 +150,84 @@ TEST(ListenerTest, StartStopListeningForEventSpec) {
ASSERT_EQ(event_sp->GetBroadcaster(), &broadcaster1);
ASSERT_FALSE(listener_sp->GetEvent(event_sp, std::chrono::seconds(0)));
}
+
+TEST(ListenerTest, MoveEventsOnHijackAndRestore) {
+ Broadcaster broadcaster(nullptr, "test-broadcaster");
+ const uint32_t event_mask = 1;
+ EventSP event_sp;
+
+ // Create the original listener and start listening.
+ ListenerSP original_listener = Listener::MakeListener("original-listener");
+ ASSERT_EQ(event_mask, original_listener->StartListeningForEvents(&broadcaster,
+ event_mask));
+ broadcaster.SetPrimaryListener(original_listener);
+
+ // Queue two events to original listener, but do not consume them yet.
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+
+ // Hijack.
+ ListenerSP hijack_listener = Listener::MakeListener("hijack-listener");
+ broadcaster.HijackBroadcaster(hijack_listener, event_mask);
+
+ // The events should have been moved to the hijack listener.
+ EXPECT_FALSE(original_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+
+ // Queue two events while hijacked, but do not consume them yet.
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+
+ // Restore the original listener.
+ broadcaster.RestoreBroadcaster();
+
+ // The events queued while hijacked should have been moved back to the
+ // original listener.
+ EXPECT_FALSE(hijack_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(original_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(original_listener->GetEvent(event_sp, std::chrono::seconds(0)));
+}
+
+TEST(ListenerTest, MoveEventsBetweenHijackers) {
+ Broadcaster broadcaster(nullptr, "test-broadcaster");
+ const uint32_t event_mask = 1;
+ EventSP event_sp;
+
+ // Create the original listener and start listening.
+ ListenerSP original_listener = Listener::MakeListener("original-listener");
+ ASSERT_EQ(event_mask, original_listener->StartListeningForEvents(&broadcaster,
+ event_mask));
+ broadcaster.SetPrimaryListener(original_listener);
+
+ // First hijack.
+ ListenerSP hijack_listener1 = Listener::MakeListener("hijack-listener1");
+ broadcaster.HijackBroadcaster(hijack_listener1, event_mask);
+
+ // Queue two events while hijacked, but do not consume
+ // them yet.
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+
+ // Second hijack.
+ ListenerSP hijack_listener2 = Listener::MakeListener("hijack-listener2");
+ broadcaster.HijackBroadcaster(hijack_listener2, event_mask);
+
+ // The second hijacker should have the events now.
+ EXPECT_FALSE(hijack_listener1->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener2->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener2->GetEvent(event_sp, std::chrono::seconds(0)));
+
+ // Queue two events while hijacked with second hijacker, but do not consume
+ // them yet.
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+ broadcaster.BroadcastEvent(event_mask, nullptr);
+
+ // Restore the previous hijacker.
+ broadcaster.RestoreBroadcaster();
+
+ // The first hijacker should now have the events.
+ EXPECT_FALSE(hijack_listener2->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener1->GetEvent(event_sp, std::chrono::seconds(0)));
+ EXPECT_TRUE(hijack_listener1->GetEvent(event_sp, std::chrono::seconds(0)));
+}
More information about the lldb-commits
mailing list