[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum added a comment.

As far as I see the problem lies in Process::ProcessEventData::DoOnRemoval:

  StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
        if (stop_info_sp && stop_info_sp->IsValid()) {
          does_anybody_have_an_opinion = true;
          bool this_thread_wants_to_stop;
          if (stop_info_sp->GetOverrideShouldStop()) {
            this_thread_wants_to_stop =
                stop_info_sp->GetOverriddenShouldStopValue();
          } else {
            stop_info_sp->PerformAction(event_ptr);
            // The stop action might restart the target.  If it does, then we
            // want to mark that in the event so that whoever is receiving it
            // will know to wait for the running event and reflect that state
            // appropriately. We also need to stop processing actions, since they
            // aren't expecting the target to be running.
  
            // FIXME: we might have run.
            if (stop_info_sp->HasTargetRunSinceMe()) {
              SetRestarted(true);
              break;
            }
  
            this_thread_wants_to_stop = stop_info_sp->ShouldStop(event_ptr);
          }
  
          if (!still_should_stop)
            still_should_stop = this_thread_wants_to_stop;
        }
      }

As you can see we get StopInfo from all the the threads available even suspended (note that all thread's stop_info are valid at this moment due to GetPrivateStopInfo gets called prior to DoOnRemoval). As a result we have a situation when suspended thread's stop_info tells we should stop even when the thread that is a real reason of stop says we should not.  Maybe you are right and the right place for the fix is inside Process::ProcessEventData::DoOnRemoval, something like this:

  if (stop_info_sp && stop_info_sp->IsValid() && thread_sp->ShouldStop()) {
  .....
  }
  }

But you know, I don't know if it possible to apply it, semantics of Thread::ShouldStop is Thread::ShouldStop(Event *) and it is unclear what kind of event to pass in. In any case, maybe I don't see the whole picture of what's going on yet but I don't see any reason to hold on stop_info of suspended thread.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80112/new/

https://reviews.llvm.org/D80112