================
@@ -582,31 +531,92 @@ bool ThreadList::WillResume() {
// There are two special kinds of thread that have priority for "StopOthers":
// a "ShouldRunBeforePublicStop thread, or the currently selected thread. If
// we find one satisfying that critereon, put it here.
- ThreadSP stop_others_thread_sp;
-
+ ThreadSP thread_to_run;
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp->GetResumeState() != eStateSuspended &&
thread_sp->GetCurrentPlan()->StopOthers()) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
+ if (thread_sp->IsOperatingSystemPluginThread() &&
+ !thread_sp->GetBackingThread())
continue;
// You can't say "stop others" and also want yourself to be suspended.
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
run_me_only_list.AddThread(thread_sp);
if (thread_sp == GetSelectedThread())
- stop_others_thread_sp = thread_sp;
-
+ thread_to_run = thread_sp;
+
if (thread_sp->ShouldRunBeforePublicStop()) {
// This takes precedence, so if we find one of these, service it:
- stop_others_thread_sp = thread_sp;
+ thread_to_run = thread_sp;
break;
}
}
}
+ if (run_me_only_list.GetSize(false) > 0 && !thread_to_run) {
+ if (run_me_only_list.GetSize(false) == 1) {
+ thread_to_run = run_me_only_list.GetThreadAtIndex(0);
+ } else {
+ int random_thread =
+ (int)((run_me_only_list.GetSize(false) * (double)rand()) /
+ (RAND_MAX + 1.0));
+ thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
+ }
+ }
+
+ // Give all the threads that are likely to run a last chance to set up their
+ // state before we negotiate who is actually going to get a chance to run...
+ // Don't set to resume suspended threads, and if any thread wanted to stop
+ // others, only call setup on the threads that request StopOthers...
+ bool wants_solo_run = run_me_only_list.GetSize(false) > 0;
+ for (pos = m_threads.begin(); pos != end; ++pos) {
----------------
jimingham wrote:
I must admit I find this version harder to reason about than the original.
Particularly, I find this second loop confusing. You have already figured out
whether you are going to run solo and which thread you are going to run, by the
time you get to this point. Then it looks like you do that computation a
second time in a slightly different way, overwriting the decisions you made
above in a fairly head-scratching way.
I would have expected at this point that if `thread_to_run` is not empty, you
could just call SetupForResume on that one thread, and then just that thread
will run. It's not at all clear to me what the purpose of redoing the
computation is.
There's one difference between the two computations. In the first loop, you
give every thread that wants to run solo a chance to be the one to do that and
pick among them equally. Then in this loop if the thread you chose to run
doesn't return `true` from `SetupForResume` (because it isn't at a breakpoint)
but there's another wants to run solo thread that is at a breakpoint, we switch
to that one.
You are changing behavior here because you prioritize threads that need to step
over breakpoints in a way that we didn't before, but the logic seems unclear to
me.
Can you say in words what behaviors you are intending to impose with this extra
"am I stepping over a breakpoint" check?
https://github.com/llvm/llvm-project/pull/120817
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
