Applying reviewer suggestions and updating the existing tests to ensure output produced by the debuggee is not affected by the output redirection.

Author: ashgti

lldb/test/API/tools/lldb-dap/output/TestDAP_output.py

@@ -10,23 +10,33 @@
 class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
     @skipIfWindows
     def test_output(self):
+        """
+        Test output handling for the running process.
+        """
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program)
+        self.build_and_launch(
+            program,
+            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)",
+                "?script print('err\\0\\0', end='\\r\\n', file=sys.stderr)",
+            ],
+        )
         source = "main.c"
         lines = [line_number(source, "// breakpoint 1")]
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.continue_to_breakpoints(breakpoint_ids)
-        
+
         # Ensure partial messages are still sent.
         output = self.collect_stdout(timeout_secs=1.0, pattern="abcdef")
-        self.assertTrue(output and len(output) > 0, "expect no program output")
+        self.assertTrue(output and len(output) > 0, "expect program stdout")
 
         self.continue_to_exit()
-        
+
         output += self.get_stdout(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
-        self.assertTrue(output and len(output) > 0, "expect no program output")
+        self.assertTrue(output and len(output) > 0, "expect program stdout")
         self.assertIn(
-            "abcdefghi\r\nhello world\r\n",
+            "abcdefghi\r\nhello world\r\nfinally\0\0out\0\0\r\nerr\0\0",
             output,
-            'full output not found in: ' + output,
+            "full stdout not found in: " + repr(output),
         )

lldb/test/API/tools/lldb-dap/output/main.c

@@ -8,5 +8,8 @@ int main() {
   printf("def");
   printf("ghi\n");
   printf("hello world\n"); // breakpoint 1
+  // Ensure the OutputRedirector does not consume the programs \0\0 output.
+  char buf[] = "finally\0";
+  write(STDOUT_FILENO, buf, sizeof(buf));
   return 0;
 }

lldb/tools/lldb-dap/OutputRedirector.cpp

@@ -19,16 +19,9 @@
 #include <unistd.h>
 #endif
 
-using lldb_private::Pipe;
-using llvm::createStringError;
-using llvm::Error;
-using llvm::Expected;
-using llvm::inconvertibleErrorCode;
-using llvm::StringRef;
+using namespace llvm;
 
-namespace {
-char kCloseSentinel[] = "\0";
-} // namespace
+static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0");
 
 namespace lldb_dap {
 
@@ -63,19 +56,19 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
     char buffer[OutputBufferSize];
     while (!m_stopped) {
       ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
-      // EOF detected.
-      if (bytes_count == 0 ||
-          (bytes_count == sizeof(kCloseSentinel) &&
-           std::memcmp(buffer, kCloseSentinel, sizeof(kCloseSentinel)) == 0 &&
-           m_fd == kInvalidDescriptor))
-        break;
       if (bytes_count == -1) {
         // Skip non-fatal errors.
         if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK)
           continue;
         break;
       }
 
+      StringRef data(buffer, bytes_count);
+      if (m_stopped)
+        data.consume_back(kCloseSentinel);
+      if (data.empty())
+        break;
+
       callback(StringRef(buffer, bytes_count));
     }
     ::close(read_fd);
@@ -93,7 +86,7 @@ void OutputRedirector::Stop() {
     // Closing the pipe may not be sufficient to wake up the thread in case the
     // write descriptor is duplicated (to stdout/err or to another process).
     // Write a null byte to ensure the read call returns.
-    (void)::write(fd, kCloseSentinel, sizeof(kCloseSentinel));
+    (void)::write(fd, kCloseSentinel.data(), kCloseSentinel.size());
     ::close(fd);
     m_forwarder.join();
   }

lldb/tools/lldb-dap/OutputRedirector.h

@@ -9,7 +9,6 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
 #define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
 
-#include "lldb/Host/Pipe.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include <atomic>