[Lldb-commits] [lldb] 6e28700 - [lldb-dap] Improving EOF handling on stream input and adding new unit tests (#129581)

[via lldb-commits]

Author: John Harrison
Date: 2025-03-04T10:09:28-08:00
New Revision: 6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4

URL: https://github.com/llvm/llvm-project/commit/6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4
DIFF: https://github.com/llvm/llvm-project/commit/6e28700ab1d876a9b01647782ce3c0ed4d8e0bb4.diff

LOG: [lldb-dap] Improving EOF handling on stream input and adding new unit tests (#129581)

This should improve the handling of EOF on stdin and adding some new
unit tests to malformed requests.

Added: 
    lldb/test/API/tools/lldb-dap/io/TestDAP_io.py

Modified: 
    lldb/tools/lldb-dap/DAP.cpp
    lldb/tools/lldb-dap/IOStream.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
new file mode 100644
index 0000000000000..a39bd17ceb3b3
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
@@ -0,0 +1,72 @@
+"""
+Test lldb-dap IO handling.
+"""
+
+from lldbsuite.test.decorators import *
+import lldbdap_testcase
+import dap_server
+
+
+class TestDAP_io(lldbdap_testcase.DAPTestCaseBase):
+    def launch(self):
+        log_file_path = self.getBuildArtifact("dap.txt")
+        process, _ = dap_server.DebugAdapterServer.launch(
+            executable=self.lldbDAPExec, log_file=log_file_path
+        )
+
+        def cleanup():
+            # If the process is still alive, terminate it.
+            if process.poll() is None:
+                process.terminate()
+                stdout_data = process.stdout.read()
+                stderr_data = process.stderr.read()
+                print("========= STDOUT =========")
+                print(stdout_data)
+                print("========= END =========")
+                print("========= STDERR =========")
+                print(stderr_data)
+                print("========= END =========")
+                print("========= DEBUG ADAPTER PROTOCOL LOGS =========")
+                with open(log_file_path, "r") as file:
+                    print(file.read())
+                print("========= END =========")
+
+        # Execute the cleanup function during test case tear down.
+        self.addTearDownHook(cleanup)
+
+        return process
+
+    def test_eof_immediately(self):
+        """
+        lldb-dap handles EOF without any other input.
+        """
+        process = self.launch()
+        process.stdin.close()
+        self.assertEqual(process.wait(timeout=5.0), 0)
+
+    def test_partial_header(self):
+        """
+        lldb-dap handles parital message headers.
+        """
+        process = self.launch()
+        process.stdin.write(b"Content-Length: ")
+        process.stdin.close()
+        self.assertEqual(process.wait(timeout=5.0), 0)
+
+    def test_incorrect_content_length(self):
+        """
+        lldb-dap handles malformed content length headers.
+        """
+        process = self.launch()
+        process.stdin.write(b"Content-Length: abc")
+        process.stdin.close()
+        self.assertEqual(process.wait(timeout=5.0), 0)
+
+    def test_partial_content_length(self):
+        """
+        lldb-dap handles partial messages.
+        """
+        process = self.launch()
+        process.stdin.write(b"Content-Length: 10{")
+        process.stdin.close()
+        self.assertEqual(process.wait(timeout=5.0), 0)

diff  --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 53c514b790f38..3dc9d6f5ca0a4 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -856,7 +856,11 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
 }
 
 llvm::Error DAP::Loop() {
-  auto cleanup = llvm::make_scope_exit([this]() { StopEventHandlers(); });
+  auto cleanup = llvm::make_scope_exit([this]() {
+    if (output.descriptor)
+      output.descriptor->Close();
+    StopEventHandlers();
+  });
   while (!disconnecting) {
     llvm::json::Object object;
     lldb_dap::PacketStatus status = GetNextObject(object);

diff  --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index c6f1bfaf3b799..ee22a297ec248 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -35,16 +35,23 @@ bool InputStream::read_full(std::ofstream *log, size_t length,
   if (status.Fail())
     return false;
 
-  text += data;
+  text += data.substr(0, length);
   return true;
 }
 
 bool InputStream::read_line(std::ofstream *log, std::string &line) {
   line.clear();
   while (true) {
-    if (!read_full(log, 1, line))
+    std::string next;
+    if (!read_full(log, 1, next))
       return false;
 
+    // If EOF is encoutnered, '' is returned, break out of this loop.
+    if (next.empty())
+      return false;
+
+    line += next;
+
     if (llvm::StringRef(line).ends_with("\r\n"))
       break;
   }
@@ -60,6 +67,7 @@ bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
     if (log)
       *log << "Warning: Expected '" << expected.str() << "', got '" << result
            << "\n";
+    return false;
   }
   return true;
 }