[Lldb-commits] [lldb] [lldb][gdb-remote] Mark thread as stopped in RefreshStateAfterStop (PR #201605)

[via lldb-commits]

================
@@ -271,6 +271,9 @@ void ThreadGDBRemote::WillResume(StateType resume_state) {
 }
 
 void ThreadGDBRemote::RefreshStateAfterStop() {
----------------
jimingham wrote:

This should be altogether wrong, because when a thread has stopped, you don't know whether it is eStateStopped or eStateCrashed.  But it turns out that for right now the Thread::GetState we really only care about running/stopped/suspended.  The only time we check this I could find we're just doing:

StateIsRunningState(thread_sp->GetState());

But for instance, if we ever get to having "non-stop" threads, then we'll want to manage that at the ThreadList layer, so the thread plugins shouldn't be themselves setting the state.  Really, the comment above that usage in ThreadList::DidStop should also apply to RefreshStateAfterStop.

So this seems to me the sort of thing that ThreadList::RefreshStateAfterStop should manage.  It's the only one that will know how to set these correctly.  It might even be more regular in the current code to do it like:

```
  collection::iterator pos, end = m_threads.end();
  for (pos = m_threads.begin(); pos != end; ++pos) {
    (*pos)->SetState(eStateStopped);
    (*pos)->RefreshStateAfterStop();
  }
```

That way by the time you get to the Thread override of RefreshStateAfterStop, you could rely on the value of GetState rather than having to figure out what it should be.

https://github.com/llvm/llvm-project/pull/201605