[lldb-dap] Restore the override FD used by the output redirect on stop.

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

@@ -18,18 +18,19 @@ 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 =========")
+                process.wait()
+            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)

lldb/tools/lldb-dap/DAP.cpp

@@ -195,34 +195,16 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
 llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
   in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true);
 
-  if (auto Error = out.RedirectTo([this](llvm::StringRef output) {
+  if (auto Error = out.RedirectTo(overrideOut, [this](llvm::StringRef output) {
         SendOutput(OutputType::Stdout, output);
       }))
     return Error;
 
-  if (overrideOut) {
-    auto fd = out.GetWriteFileDescriptor();
-    if (auto Error = fd.takeError())
-      return Error;
-
-    if (dup2(*fd, fileno(overrideOut)) == -1)
-      return llvm::errorCodeToError(llvm::errnoAsErrorCode());
-  }
-
-  if (auto Error = err.RedirectTo([this](llvm::StringRef output) {
+  if (auto Error = err.RedirectTo(overrideErr, [this](llvm::StringRef output) {
         SendOutput(OutputType::Stderr, output);
       }))
     return Error;
 
-  if (overrideErr) {
-    auto fd = err.GetWriteFileDescriptor();
-    if (auto Error = fd.takeError())
-      return Error;
-
-    if (dup2(*fd, fileno(overrideErr)) == -1)
-      return llvm::errorCodeToError(llvm::errnoAsErrorCode());
-  }
-
   return llvm::Error::success();
 }
 
@@ -729,15 +711,11 @@ PacketStatus DAP::GetNextObject(llvm::json::Object &object) {
 
   llvm::StringRef json_sref(json);
   llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref);
-  if (!json_value) {
-    auto error = json_value.takeError();
-    if (log) {
-      std::string error_str;
-      llvm::raw_string_ostream strm(error_str);
-      strm << error;
+  if (auto error = json_value.takeError()) {
+    std::string error_str = llvm::toString(std::move(error));
+    if (log)
       *log << "error: failed to parse JSON: " << error_str << std::endl
            << json << std::endl;
-    }
     return PacketStatus::JSONMalformed;
   }
 
@@ -848,19 +826,15 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
 
   SendTerminatedEvent();
 
-  // Stop forwarding the debugger output and error handles.
-  out.Stop();
-  err.Stop();
-
   disconnecting = true;
 
   return error;
 }
 
 llvm::Error DAP::Loop() {
   auto cleanup = llvm::make_scope_exit([this]() {
-    if (output.descriptor)
-      output.descriptor->Close();
+    out.Stop();
+    err.Stop();
     StopEventHandlers();
   });
   while (!disconnecting) {

lldb/tools/lldb-dap/OutputRedirector.cpp

@@ -27,7 +27,9 @@ namespace lldb_dap {
 
 int OutputRedirector::kInvalidDescriptor = -1;
 
-OutputRedirector::OutputRedirector() : m_fd(kInvalidDescriptor) {}
+OutputRedirector::OutputRedirector()
+    : m_fd(kInvalidDescriptor), m_original_fd(kInvalidDescriptor),
+      m_restore_fd(kInvalidDescriptor) {}
 
 Expected<int> OutputRedirector::GetWriteFileDescriptor() {
   if (m_fd == kInvalidDescriptor)
@@ -36,7 +38,8 @@ Expected<int> OutputRedirector::GetWriteFileDescriptor() {
   return m_fd;
 }
 
-Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
+Error OutputRedirector::RedirectTo(std::FILE *file_override,
+                                   std::function<void(StringRef)> callback) {
   assert(m_fd == kInvalidDescriptor && "Output readirector already started.");
   int new_fd[2];
 
@@ -52,6 +55,19 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
 
   int read_fd = new_fd[0];
   m_fd = new_fd[1];
+
+  if (override) {
+    int override_fd = fileno(override);
+
+    // Backup the FD to restore once redirection is complete.
+    m_original_fd = override_fd;
+    m_restore_fd = dup(override_fd);
+
+    // Override the existing fd the new write end of the pipe.
+    if (::dup2(m_fd, override_fd) == -1)
+      return llvm::errorCodeToError(llvm::errnoAsErrorCode());
+  }
+
   m_forwarder = std::thread([this, callback, read_fd]() {
     char buffer[OutputBufferSize];
     while (!m_stopped) {
@@ -92,6 +108,17 @@ void OutputRedirector::Stop() {
     (void)::write(fd, kCloseSentinel.data(), kCloseSentinel.size());
     ::close(fd);
     m_forwarder.join();
+
+    // Restore the fd back to its original state since we stopped the
+    // redirection.
+    if (m_restore_fd != kInvalidDescriptor &&
+        m_original_fd != kInvalidDescriptor) {
+      int restore_fd = m_restore_fd;
+      m_restore_fd = kInvalidDescriptor;
+      int original_fd = m_original_fd;
+      m_original_fd = kInvalidDescriptor;
+      ::dup2(restore_fd, original_fd);
+    }
   }
 }
 

lldb/tools/lldb-dap/OutputRedirector.h

@@ -27,7 +27,8 @@ class OutputRedirector {
   /// \return
   ///     \a Error::success if the redirection was set up correctly, or an error
   ///     otherwise.
-  llvm::Error RedirectTo(std::function<void(llvm::StringRef)> callback);
+  llvm::Error RedirectTo(std::FILE *overrideFile,
+                         std::function<void(llvm::StringRef)> callback);
 
   llvm::Expected<int> GetWriteFileDescriptor();
   void Stop();
@@ -41,6 +42,8 @@ class OutputRedirector {
 private:
   std::atomic<bool> m_stopped = false;
   int m_fd;
+  int m_original_fd;
+  int m_restore_fd;
   std::thread m_forwarder;
 };