Skip to content

[lldb] Fix new thread detection mechanism #10064

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

Conversation

felipepiovezan
Copy link

In ThreadList::WillResume, we scan the list of threads to check whether a solo run was requested. If so, we ensure that new threads can't be created and run.

However, we need to make these checks after SetupForResume is called, as that may push plans. For example, a thread that has only a Base Plan, but has hit a breakpoint, will enqueue a "Step Over Breakpoint" plan, which must be run in isolation.

Unfortunately, it seems impossible to write a test for this.

Note: this was "fixed" by this "NFC" commit upstream: llvm#120817
I don't want to cherry pick such a major patch here; instead, I'm proposing this more targeted fix that we don't need to put into next.

@felipepiovezan
Copy link
Author

@swift-ci test

@@ -151,6 +151,11 @@ class ThreadList : public ThreadCollection {
std::vector<lldb::tid_t> m_expression_tid_stack;

private:
/// Returns true if the current ThreadPlan of any threads requested
/// StopOthers. This ignores OS Plugin threads that are not currently backed
/// by any real thread.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this restriction necessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the authors original intent was that "if there is no backing thread, this thread is probably not going to run?". I'm not sure

…cution (llvm#120817)

These changes are designed to not change any behavior, but to make it
easy to add code to choose the direction of execution after we've
identified which thread(s) to run but before we add any
`ThreadPlanStepOverBreakpoint`s. And honestly I think they make the
existing code a bit clearer.

(cherry picked from commit d594d4c)
@felipepiovezan felipepiovezan force-pushed the felipe/new_thread_detection_mechanism branch from 8cbf27d to 45ee7b8 Compare February 19, 2025 23:21
@felipepiovezan
Copy link
Author

Converted this PR into a cherry-pick of the upstream commit.

@felipepiovezan
Copy link
Author

@swift-ci test

@felipepiovezan felipepiovezan merged commit 3c84c55 into swiftlang:stable/20240723 Feb 20, 2025
3 checks passed
@felipepiovezan felipepiovezan deleted the felipe/new_thread_detection_mechanism branch February 20, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants