JDevlieghere wrote: In the PR description you said "I don't think it's reasonable to allow getting threads from a running process." and I agree with that. However, that's not what `lldb-dap` does right now. The request handler calls `SBProcess::GetNumThreads` and `SBProcess::GetThreadAtIndex` without checking that the process is stopped, or trying to interrupt the process. Under the hood these two methods use the StopLocker to check if the process is stopped and only update the thread list if it is.
The current implementation is incorrect and unsafe in multiple ways: 1. If we get a Thread request while the process is running, we're going to return stale data from the last stop. The protocol doesn't actually specify that this request returns the current threads (as opposed to the last threads the adapter knows about), so maybe this part is fine. 2. If we get a Threads request while the process is running, and it stops (e.g. crashes) while we're handling the request, we're going to reply with partially state data. The threads we're iterating over until the stop will be stale and the ones after the stop will be active. And if the number of threads changed we'll get that wrong too. 3. The same problem exists if the process were to resume, but I don't think that's possible, except maybe due to a stop hook or expression evaluation. I created a draft PR to stop the process before we get the threads, but that's not sufficient. We need proper locking, like we added for #131242, but this time to protect the process control. That probably requires exposing the `StopLocker` through the SB API and having a method that halts the process and returns the StopLocker, which lldb-dap then can hold onto until it's done. https://github.com/llvm/llvm-project/pull/134339 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
