[Lldb-commits] [lldb] [RFC][lldb-dap] Always stop on enrty for attaching (PR #134339)

[Greg Clayton via lldb-commits]

clayborg wrote:

> That code has been there at least since the dread reformatting.
> 
> I have no idea why past one of us thought it was a good idea to return the previous number of threads or the old thread list when you weren't stopped. We don't do that with most anything else you ask about threads, and I can't think of any way that would be useful.
> 
> Whatever we do with DAP, we should stop doing this in SBThread...

I am totally fine updating `SBProcess::GetNumThreads()` and `SBProcess::GetThreadAtIndex()` to fully listen to the stop locker and return 0 and empty SBThread objects. 

So the fix now should be we modify `SBProcess::GetNumThreads()` and `SBProcess::GetThreadAtIndex()` to listen to the thread locker. Old code:
```
SBThread SBProcess::GetThreadAtIndex(size_t index) {
  LLDB_INSTRUMENT_VA(this, index);

  SBThread sb_thread;
  ThreadSP thread_sp;
  ProcessSP process_sp(GetSP());
  if (process_sp) {
    Process::StopLocker stop_locker;
    const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock());
    std::lock_guard<std::recursive_mutex> guard(
        process_sp->GetTarget().GetAPIMutex());
    thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, can_update);
    sb_thread.SetThread(thread_sp);
  }

  return sb_thread;
}
```
New code should be:
```
SBThread SBProcess::GetThreadAtIndex(size_t index) {
  LLDB_INSTRUMENT_VA(this, index);

  SBThread sb_thread;
  ThreadSP thread_sp;
  ProcessSP process_sp(GetSP());
  if (process_sp) {
    Process::StopLocker stop_locker;
    if (stop_locker.TryLock(&process_sp->GetRunLock())) {
      std::lock_guard<std::recursive_mutex> guard(
          process_sp->GetTarget().GetAPIMutex());
      thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, false);
      sb_thread.SetThread(thread_sp);
    }
  }
  return sb_thread;
}
```

Then we also need to sync in :

Old code:
```
void ConfigurationDoneRequestHandler::operator()(
    const llvm::json::Object &request) const {
  llvm::json::Object response;
  FillResponse(request, response);
  dap.SendJSON(llvm::json::Value(std::move(response)));
  dap.configuration_done_sent = true;
  if (dap.stop_at_entry)
    SendThreadStoppedEvent(dap);
  else
    dap.target.GetProcess().Continue();
}
```
New code:
```
void ConfigurationDoneRequestHandler::operator()(
    const llvm::json::Object &request) const {
  llvm::json::Object response;
  FillResponse(request, response);
  dap.SendJSON(llvm::json::Value(std::move(response)));
  dap.configuration_done_sent = true;
  if (dap.stop_at_entry)
    SendThreadStoppedEvent(dap);
  else
    dap.SynchronousContinue();
}
```

Where `dap.SynchronousContinue()` will send the `dap.target.GetProcess().Continue()` and then make sure we consume the `eStateRunning` event to ensure we have the process running and that run locker will work as expected.

Does that sound ok to everyone?

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