[Lldb-commits] [lldb] [lldb-dap] Addressing the order of events during disconnect to flush output. (PR #128583)

[John Harrison via lldb-commits]

https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128583

>From 176eae365226716a2a2ead2f6de21d801238cf1b Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 24 Feb 2025 13:35:29 -0800
Subject: [PATCH 1/2] [lldb-dap] Addressing the order of events during
 disconnect to ensure we shutdown the debugger output handle and error handle
 redirection before disconnect completes.

The TestDAP_ouput test is flaky due to the order of events during shutdown.  We were stopping the output and error handle redirection after we finished the disconnect request, which can cause us to miss output events due to timing. Moving when we stop the redirection to ensure we have consistent output prior to disconnect responding.
---
 lldb/test/API/tools/lldb-dap/output/TestDAP_output.py |  4 ++++
 lldb/tools/lldb-dap/DAP.cpp                           | 11 ++++++-----
 lldb/tools/lldb-dap/DAP.h                             |  4 ++--
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
index eae7629409844..fe54511d1e21f 100644
--- a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
+++ b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
@@ -16,6 +16,7 @@ def test_output(self):
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(
             program,
+            disconnectAutomatically=False,
             exitCommands=[
                 # Ensure that output produced by lldb itself is not consumed by the OutputRedirector.
                 "?script print('out\\0\\0', end='\\r\\n', file=sys.stdout)",
@@ -33,6 +34,9 @@ def test_output(self):
 
         self.continue_to_exit()
 
+        # Disconnecting from the server to ensure any pending IO is flushed.
+        self.dap_server.request_disconnect()
+
         output += self.get_stdout(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
         self.assertTrue(output and len(output) > 0, "expect program stdout")
         self.assertIn(
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 9b22b60a68d94..0bc570e61916c 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -223,10 +223,7 @@ llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
   return llvm::Error::success();
 }
 
-void DAP::StopIO() {
-  out.Stop();
-  err.Stop();
-
+void DAP::StopEventHandlers() {
   if (event_thread.joinable()) {
     broadcaster.BroadcastEventByType(eBroadcastBitStopEventThread);
     event_thread.join();
@@ -853,13 +850,17 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
 
   SendTerminatedEvent();
 
+  // Stop forward the debugger output and error file handle.
+  out.Stop();
+  err.Stop();
+
   disconnecting = true;
 
   return error;
 }
 
 llvm::Error DAP::Loop() {
-  auto stop_io = llvm::make_scope_exit([this]() { StopIO(); });
+  auto stop_io = llvm::make_scope_exit([this]() { StopEventHandlers(); });
   while (!disconnecting) {
     llvm::json::Object object;
     lldb_dap::PacketStatus status = GetNextObject(object);
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 18de39838f218..d0e1217aaeb8e 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -224,8 +224,8 @@ struct DAP {
   llvm::Error ConfigureIO(std::FILE *overrideOut = nullptr,
                           std::FILE *overrideErr = nullptr);
 
-  /// Stop the redirected IO threads and associated pipes.
-  void StopIO();
+  /// Stop event handler threads.
+  void StopEventHandlers();
 
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.

>From 72360fc70861247754531bff1799f46132f4c575 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 24 Feb 2025 13:42:59 -0800
Subject: [PATCH 2/2] Adjusting a variable name.

---
 lldb/tools/lldb-dap/DAP.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 0bc570e61916c..81de6e526547a 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -860,7 +860,7 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
 }
 
 llvm::Error DAP::Loop() {
-  auto stop_io = llvm::make_scope_exit([this]() { StopEventHandlers(); });
+  auto cleanup = llvm::make_scope_exit([this]() { StopEventHandlers(); });
   while (!disconnecting) {
     llvm::json::Object object;
     lldb_dap::PacketStatus status = GetNextObject(object);