-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Revert "[lldb] Fix race condition in Process::WaitForProcessToStop() … #148915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
jimingham
merged 1 commit into
llvm:main
from
jimingham:revert-process-fix-hang-on-kill
Jul 15, 2025
Merged
Revert "[lldb] Fix race condition in Process::WaitForProcessToStop() … #148915
jimingham
merged 1 commit into
llvm:main
from
jimingham:revert-process-fix-hang-on-kill
Jul 15, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lvm#144919)" This was causing a couple of failures on the Ubuntu bots. Reverting while we wait on a fix for those issues. This reverts commit 8612926.
|
@llvm/pr-subscribers-lldb Author: None (jimingham) Changes…(#144919)" This was causing a couple of failures on the Ubuntu bots. Reverting while we wait on a fix for those issues. Full diff: https://github.com/llvm/llvm-project/pull/148915.diff 4 Files Affected:
diff --git a/lldb/include/lldb/Utility/Listener.h b/lldb/include/lldb/Utility/Listener.h
index 393169091390c..d48816ec0ea4d 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -53,13 +53,6 @@ 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 41b782fee55fa..c6b2606afe0c8 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -335,18 +335,6 @@ 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;
@@ -379,19 +367,6 @@ 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 a9b733579d2f7..d4ce3bf25ec5a 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -176,33 +176,6 @@ 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 a980a9f01c488..f7aa0f59d1848 100644
--- a/lldb/unittests/Utility/ListenerTest.cpp
+++ b/lldb/unittests/Utility/ListenerTest.cpp
@@ -150,84 +150,3 @@ 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)));
-}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…(#144919)"
This was causing a couple of failures on the Ubuntu bots. Reverting while we wait on a fix for those issues.
This reverts commit 8612926.