[lldb-dap] Fix TestDap_attach.py flakiness

[kusmour]

Summary

This patch makes the request_attach wait for events process and initialized just like request_launch. This ensure the DAP session can move forward somewhat correctly.

Recently TestDap_attach.test_terminate_commands became flaky.
It's hitting:

lldbsuite/test/tools/lldb-dap/dap_server.py", line 350, in send_recv
    raise ValueError(desc)
ValueError: no response for "disconnect"

I took a look at the DAP msg from that test case and noticed:

• It's not using the regular attaching, instead it's using the attachCommands to launch debug the binary and it will stop at entry.
• The initialized event returned after the disconnect request. Which means lldb-dap didn't really get ready yet.

NOTE

The dap_server.py is doing things to mimic the VSCode (or other dap clients) but it had some assumptions. For example, it's still missing the configurationDone request and response because it relies on a continue action to trigger the configurationDone request.

Test Plan

./bin/llvm-lit -va /Users/wanyi/llvm-upstream/llvm-project/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
./bin/llvm-lit -va /Users/wanyi/llvm-upstream/llvm-project/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

To test the wait_for_events timeout case

events = self.wait_for_events(["process", "initialized", "fake", "event"], 1)
if events:
    raise ValueError(f'no events {",".join(events)} found for within timeout 1')

Observed

Also

Looks like some test cases should be re-enabled in 0b8dfb5
But only comments was removed. The skip statements survived the change.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Wanyi (kusmour)

Changes

This patch makes the request_attach wait for events process and initialized just like request_launch. This ensure the DAP session can move forward somewhat correctly.

Recently TestDap_attach.test_terminate_commands became flaky.
It's hitting:

lldbsuite/test/tools/lldb-dap/dap_server.py", line 350, in send_recv
    raise ValueError(desc)
ValueError: no response for "disconnect"

I took a look at the DAP msg from that test case and noticed:

• It's not using the regular attaching, instead it's using the attachCommands to launch debug the binary and it will stop at entry.
• The initialized event returned after the disconnect request. Which means lldb-dap didn't really get ready yet.

NOTE: The dap_server.py is doing things to mimic the VSCode (or other dap clients) but it had some assumptions. For example, it's still missing the configurationDone request and response because it relies on a continue action to trigger the configurationDone request.

---

Full diff: https://github.com/llvm/llvm-project/pull/137278.diff

1 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+7-1)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index dadf6b1f8774c..20f5286da3203 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -612,7 +612,13 @@ def request_attach(
         if gdbRemoteHostname is not None:
             args_dict["gdb-remote-hostname"] = gdbRemoteHostname
         command_dict = {"command": "attach", "type": "request", "arguments": args_dict}
-        return self.send_recv(command_dict)
+        response = self.send_recv(command_dict)
+
+        if response["success"]:
+            # Wait for a 'process' and 'initialized' event in any order
+            self.wait_for_event(filter=["process", "initialized"])
+            self.wait_for_event(filter=["process", "initialized"])
+        return response
 
     def request_breakpointLocations(
         self, file_path, line, end_line=None, column=None, end_column=None

[dmpots]

LGTM! I'm not too familiar with the dap protocol, so would be good to get another approver.

[JDevlieghere]

If we hit a timeout, wont event be None?

[dmpots]

Ah yeah it looks like its not handling the timeout correctly.

[dmpots]

Ah yeah it looks like its not handling the timeout correctly.

[DavidSpickett]

We (Linaro) are seeing the same error message with a different test on our AArch64 Linux bot:

ERROR: test_indexedVariables_with_raw_child_for_synthetics (TestDAP_variables.TestDAP_variables)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2065, in tearDown
    Base.tearDown(self)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1105, in tearDown
    hook()  # try the plain call and hope it works
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 422, in cleanup
    self.dap_server.request_disconnect(terminateDebuggee=True)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 708, in request_disconnect
    return self.send_recv(command_dict)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 338, in send_recv
    raise ValueError(desc)
ValueError: no response for "disconnect"

https://lab.llvm.org/buildbot/#/builders/59/builds/16771

Likely same cause.

[dmpots]

Do we want an explicit check that event_dict is None? Or would it be better to have a truth test like

if event_dict:
   ...

[JDevlieghere]

LGTM