[lldb-dap] Waiting for the test binary to exit prior to dumping logs.

[ashgti]

This should ensure we have the full logs prior to dumping the logs. Additionally, printing log dumps to stderr so they are adjacent to assertion failures.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

This should ensure we have the full logs prior to dumping the logs. Additionally, printing log dumps to stderr so they are adjacent to assertion failures.

---

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

1 Files Affected:

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

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 0fea3419d9725..a9a47e281e829 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
@@ -88,13 +88,13 @@ def packet_type_is(packet, packet_type):
 
 
 def dump_dap_log(log_file):
-    print("========= DEBUG ADAPTER PROTOCOL LOGS =========")
+    print("========= DEBUG ADAPTER PROTOCOL LOGS =========", file=sys.stderr)
     if log_file is None:
-        print("no log file available")
+        print("no log file available", file=sys.stderr)
     else:
         with open(log_file, "r") as file:
-            print(file.read())
-    print("========= END =========")
+            print(file.read(), file=sys.stderr)
+    print("========= END =========", file=sys.stderr)
 
 
 def read_packet_thread(vs_comm, log_file):
@@ -107,6 +107,10 @@ def read_packet_thread(vs_comm, log_file):
             # termination of lldb-dap and stop waiting for new packets.
             done = not vs_comm.handle_recv_packet(packet)
     finally:
+        # Wait for the process to fully exit before dumping the log file to
+        # ensure we have the entire log contents.
+        if vs_comm.process is not None:
+            vs_comm.process.wait()
         dump_dap_log(log_file)
 
 

[JDevlieghere]

Should we provide a reasonable timeout (e.g. 5 seconds) so that if the process hangs, we still dump the logs? Maybe even send a kill?

[walter-erquinigo]

I'd say 20 seconds. Some build bots make every test run very slowly

[JDevlieghere]

I'm fine with 20 (or really anything below 600, which is the individual lit timeout).

[ashgti]

The test itself usually handles this with the call to terminate in

llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

Line 1273 in b251c29

def terminate(self):

called from:

llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

Line 336 in b251c29

self.dap_server.terminate()

But the read_packet_thread thread is never joined or waited on so it tends to write the DAP logs whenever it thinks its done with the connection, not when the process has exited.

[JDevlieghere]

Okay, so the main thread is going to send a SIGTERM asking the process to terminate gracefully. I think the timeout still makes sense (at least here) in case it doesn't, but if we wanted to send a SIGKILL, I agree we should do it where we currently call terminate.

[ashgti]

Updated both of the waits to have a timeout of 20s.

For the read_packet_thread, we'll just print whatever logs we find after the timeout.

For the terminate call, we upgrade to a kill().

[JDevlieghere]

🚢