This adds a new 'CapabilitiesEventBody' type for having a well
structured type for the event and updates the 'restart' request
to dynamically set their capabilities.
Moving `threads` request to structured types. Adding helper types for
this and moving helpers from JSONUtils to ProtocolUtils.
---------
Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
This reverts commit 4b6c608615a285d81132acf8e33b81b2ec2c9bf9 and
follow-up commits 159de3633640a5cb2d322ebe8cc4ec0c1c9a896d and
c9e1c52e2e75a91a44a98df818cc9bd11655e51d because this breaks
TestDAP_stepInTargets.py on Darwin.
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake
This is more straight forward refactor of the startup sequence that
reverts parts of ba29e60f9a2222bd5e883579bb78db13fc5a7588. Unlike my
previous attempt, I ended up removing the pending request queue and not
including an `AsyncReqeustHandler` because I don't think we actually
need that at the moment.
The key is that during the startup flow there are 2 parallel operations
happening in the DAP that have different triggers.
* The `initialize` request is sent and once the response is received the
`launch` or `attach` is sent.
* When the `initialized` event is recieved the `setBreakpionts` and
other config requests are made followed by the `configurationDone`
event.
I moved the `initialized` event back to happen in the `PostRun` of the
`launch` or `attach` request handlers. This ensures that we have a valid
target by the time the configuration calls are made. I added also added
a few extra validations that to the `configurationeDone` handler to
ensure we're in an expected state.
I've also fixed up the tests to match the new flow. With the other
additional test fixes in 087a5d2ec7897cd99d3787820711fec76a8e1792 I
think we've narrowed down the main source of test instability that
motivated the startup sequence change.
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize
request, rather than after we finish attaching or launching.
- Delay handling the launch and attach request until we have handled
the configurationDone request. The latter is now largely a NO-OP and
only exists to signal lldb-dap that it can handle the launch and
attach requests.
- Delay handling the initial threads requests until we have handled
the launch or attach request.
- Make all attaching and launching synchronous, including when we have
attach or launch commands. This removes the need to synchronize
between the request and event thread.
Background:
https://discourse.llvm.org/t/reliability-of-the-lldb-dap-tests/86125
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize
request, rather than after we finish attaching or launching.
- Delay handling the launch and attach request until we have handled
the configurationDone request. The latter is now largely a NO-OP and
only exists to signal lldb-dap that it can handle the launch and
attach requests.
- Delay handling the initial threads requests until we have handled
the launch or attach request.
- Make all attaching and launching synchronous, including when we have
attach or launch commands. This removes the need to synchronize
between the request and event thread.
Background:
https://discourse.llvm.org/t/reliability-of-the-lldb-dap-tests/86125
# Summary
This PR updates `SBProcess::GetNumThreads()` and
`SBProcess::GetThreadAtIndex()` to listen to the stop locker.
`SBProcess::GetNumThreads()` will return 0 if the process is running.
## Problem Description
Recently upon debugging a program with thousands of threads in VS Code,
lldb-dap would hang at a `threads` request sent right after receiving
the `configurationDone` response. Soon after it will end the debug
session with the following error
```
Process <pid> exited with status = -1 (0xffffffff) lost connection
```
This is because LLDB is still in the middle of resuming all the threads.
And requesting threads will end up interrupt the process on Linux. From
the gdb-remote log it ended up getting `lldb::StateType::eStateInvalid`
and just exit with status -1.
I don't think it's reasonable to allow getting threads from a running
process. There are a few approaches to fix this:
1) Send the stopped event to IDE after `configurationDone`. This aligns
with the CLI behavior.
2) However, the above approach will break the existing user facing
behavior. The alternative will be reject the `threads` request if the
process is not stopped.
3) Improve the run lock. This is a synchronize issue where process was
in the middle of resuming while lldb-dap attempts to interrupt it.
**This PR implements the option 3**
## HOWEVER
This fixed the "lost connection" issue below but new issue has surfaced.
From testing, and also from checking the [VSCode source
code](174af221c9/src/vs/workbench/contrib/debug/browser/debugSession.ts (L791)),
it expects having threadID to perform `pause`. So after attaching,
without any threads reported to the client, the user will not be able to
pause the attached process. `setBreakpoint` will still work and once we
make a stop at the bp (or any stop that will report threads, client can
perform pause again.
## NEXT
1) Made an attempt to return initial thread list so that VSCode can
pause (second commit in the PR)
2) Investigate why threads will trigger unwinding the second frame of a
thread, which leads to sending the interrupt
3) Decided if we want to support `stopOnEntry` for attaching, given
i. This is not an official specification
ii. If enable stopOnEntry, we need to fix attaching on Linux, to send
only one stopped event. Currently, all threads upon attaching will have
stop reason `SIGSTOP` and lldb-dap will send `stopped` event for each
one of them. Every `stopped` will trigger the client request for
threads.
iii. Alternatively, we can support auto continue correspond to `(lldb)
process attach --continue`. This require the ii above.
### Additionally
lldb-dap will not send a `continued` event after `configurationDone`
because it checks `dap.focus_tid == LLDB_INVALID_THREAD_ID` (so that we
don't send it for `launch` request). Notice `dap.focus_tid` will only
get assigned when handling stop or stepping.
According to DAP
> Please note: a debug adapter is not expected to send this event in
response to a request that implies that execution continues, e.g. launch
or continue.
It is only necessary to send a continued event if there was no previous
request that implied this.
So I guess we are not violating DAP if we don't send `continued` event.
But I'd like to get some sense about this.
## Test Plan
Used following program for testing:
https://gist.github.com/kusmour/1729d2e07b7b1063897db77de194e47d
**NOTE: Utilize stdin to get pid and attach AFTER hitting enter. Attach
should happen when all the threads start running.**
DAP messages before the change
<img width="1165" alt="image"
src="https://github.com/user-attachments/assets/a9ad85fb-81ce-419c-95e5-612639905c66"
/>
DAP message after the change - report zero threads after attaching
<img width="1165" alt="image"
src="https://github.com/user-attachments/assets/a1179e18-6844-437a-938c-0383702294cd"
/>
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
Currently, all request handlers are implemented as free functions in
lldb-dap.cpp. That file has grown to over 5000 lines and is starting to
become hard to maintain. This PR moves the request handlers into their
own class (and file), together with their documentation.
This PR migrates about a third of the request handlers and the rest will
be migrated in subsequent commits. I'm merging this in an incomplete
state because almost any lldb-dap change is going to result in merge
conflicts and migrating request handlers one by one is easier to review.
