[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)

John Harrison via lldb-commits lldb-commits at lists.llvm.org
Fri Dec 20 14:25:12 PST 2024


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

>From 4131b8c5af83a219efb2513a1ac90e8b9877d2ef Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 17 Dec 2024 17:45:34 -0800
Subject: [PATCH 1/5] [lldb-dap] Ensure the IO forwarding threads are managed
 by the DAP object lifecycle.

This moves the ownership of the threads that forward stdout/stderr to the DAP object itself to ensure that the threads are joined and that the forwarding is cleaned up when the DAP connection is disconnected.

This is part of a larger refactor to allow lldb-dap to run in a listening mode and accept multiple connections.
---
 lldb/tools/lldb-dap/CMakeLists.txt       |   6 +-
 lldb/tools/lldb-dap/DAP.cpp              | 114 ++++++++++++++----
 lldb/tools/lldb-dap/DAP.h                |  67 +++++++----
 lldb/tools/lldb-dap/IOStream.cpp         |   8 +-
 lldb/tools/lldb-dap/IOStream.h           |  14 ++-
 lldb/tools/lldb-dap/OutputRedirector.cpp |  63 ----------
 lldb/tools/lldb-dap/OutputRedirector.h   |  26 ----
 lldb/tools/lldb-dap/lldb-dap.cpp         | 146 +++++++++++++----------
 8 files changed, 231 insertions(+), 213 deletions(-)
 delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.cpp
 delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.h

diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index d68098bf7b3266..00906e8ac10904 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD" )
-  list(APPEND extra_libs lldbHost)
-endif ()
-
 if (HAVE_LIBPTHREAD)
   list(APPEND extra_libs pthread)
 endif ()
@@ -32,7 +28,6 @@ add_lldb_tool(lldb-dap
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
-  OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
@@ -42,6 +37,7 @@ add_lldb_tool(lldb-dap
 
   LINK_LIBS
     liblldb
+    lldbHost
     ${extra_libs}
 
   LINK_COMPONENTS
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..4e0de6fa3a4e90 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -6,34 +6,53 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include <chrono>
-#include <cstdarg>
-#include <fstream>
-#include <mutex>
-
 #include "DAP.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
+#include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include <algorithm>
+#include <cassert>
+#include <chrono>
+#include <cstdarg>
+#include <cstdio>
+#include <fstream>
+#include <mutex>
+#include <utility>
 
 #if defined(_WIN32)
 #define NOMINMAX
 #include <fcntl.h>
 #include <io.h>
 #include <windows.h>
+#else
+#include <unistd.h>
 #endif
 
 using namespace lldb_dap;
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
-    : debug_adaptor_path(path), broadcaster("lldb-dap"),
+DAP::DAP(llvm::StringRef path, std::optional<std::ofstream> &log,
+         ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output)
+    : debug_adaptor_path(path), log(log), input(std::move(input)),
+      output(std::move(output)), broadcaster("lldb-dap"),
       exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
       stop_at_entry(false), is_attach(false),
       enable_auto_variable_summaries(false),
@@ -43,21 +62,7 @@ DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
       configuration_done_sent(false), waiting_for_run_in_terminal(false),
       progress_event_reporter(
           [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
-      reverse_request_seq(0), repl_mode(repl_mode) {
-  const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
-  // Windows opens stdout and stdin in text mode which converts \n to 13,10
-  // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
-  // fixes this.
-  int result = _setmode(fileno(stdout), _O_BINARY);
-  assert(result);
-  result = _setmode(fileno(stdin), _O_BINARY);
-  UNUSED_IF_ASSERT_DISABLED(result);
-  assert(result);
-#endif
-  if (log_file_path)
-    log.reset(new std::ofstream(log_file_path));
-}
+      reverse_request_seq(0), repl_mode(repl_mode) {}
 
 DAP::~DAP() = default;
 
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
   return nullptr;
 }
 
+llvm::Error DAP::ConfigureIO(std::optional<std::FILE *> overrideOut,
+                             std::optional<std::FILE *> overrideErr) {
+  auto *inull = lldb_private::FileSystem::Instance().Fopen(
+      lldb_private::FileSystem::DEV_NULL, "w");
+  in = lldb::SBFile(inull, true);
+
+  lldb_private::Status status;
+  status = pout.CreateNew(/*child_process_inherit=*/false);
+  if (status.Fail())
+    return status.takeError();
+  status = perr.CreateNew(/*child_process_inherit=*/false);
+  if (status.Fail())
+    return status.takeError();
+
+  if (overrideOut) {
+    if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+      return llvm::make_error<llvm::StringError>(
+          llvm::errnoAsErrorCode(),
+          llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+              .str()
+              .c_str());
+    }
+  }
+
+  if (overrideErr) {
+    if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+      return llvm::make_error<llvm::StringError>(
+          llvm::errnoAsErrorCode(),
+          llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+              .str()
+              .c_str());
+    }
+  }
+
+  auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
+    char buffer[4098];
+    size_t bytes_read;
+    while (pipe.CanRead()) {
+      lldb_private::Status error = pipe.ReadWithTimeout(
+          &buffer, sizeof(buffer), std::chrono::seconds(1), bytes_read);
+      if (error.Success()) {
+        // zero bytes returned on EOF.
+        if (bytes_read == 0)
+          break;
+        SendOutput(outputType, llvm::StringRef(buffer, bytes_read));
+      }
+    }
+  };
+
+  stdout_forward_thread =
+      std::thread(forwarder, std::ref(pout), OutputType::Stdout);
+  stderr_forward_thread =
+      std::thread(forwarder, std::ref(perr), OutputType::Stderr);
+
+  return llvm::Error::success();
+}
+
 // Send the JSON in "json_str" to the "out" stream. Correctly send the
 // "Content-Length:" field followed by the length, followed by the raw
 // JSON bytes.
@@ -208,19 +270,19 @@ std::string DAP::ReadJSON() {
   std::string json_str;
   int length;
 
-  if (!input.read_expected(log.get(), "Content-Length: "))
+  if (!input.read_expected(log, "Content-Length: "))
     return json_str;
 
-  if (!input.read_line(log.get(), length_str))
+  if (!input.read_line(log, length_str))
     return json_str;
 
   if (!llvm::to_integer(length_str, length))
     return json_str;
 
-  if (!input.read_expected(log.get(), "\r\n"))
+  if (!input.read_expected(log, "\r\n"))
     return json_str;
 
-  if (!input.read_full(log.get(), length, json_str))
+  if (!input.read_full(log, length, json_str))
     return json_str;
 
   if (log) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index ae496236f13369..f9cedd32cd0bd3 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -9,36 +9,38 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_DAP_H
 #define LLDB_TOOLS_LLDB_DAP_DAP_H
 
-#include <cstdio>
-#include <iosfwd>
-#include <map>
-#include <optional>
-#include <thread>
-
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/JSON.h"
-#include "llvm/Support/Threading.h"
-#include "llvm/Support/raw_ostream.h"
-
-#include "lldb/API/SBAttachInfo.h"
-#include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
-#include "lldb/API/SBDebugger.h"
-#include "lldb/API/SBEvent.h"
-#include "lldb/API/SBFormat.h"
-#include "lldb/API/SBLaunchInfo.h"
-#include "lldb/API/SBTarget.h"
-#include "lldb/API/SBThread.h"
-
+#include "DAPForward.h"
 #include "ExceptionBreakpoint.h"
 #include "FunctionBreakpoint.h"
 #include "IOStream.h"
 #include "InstructionBreakpoint.h"
 #include "ProgressEvent.h"
 #include "SourceBreakpoint.h"
+#include "lldb/API/SBBroadcaster.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFile.h"
+#include "lldb/API/SBFormat.h"
+#include "lldb/API/SBFrame.h"
+#include "lldb/API/SBTarget.h"
+#include "lldb/API/SBThread.h"
+#include "lldb/API/SBValue.h"
+#include "lldb/API/SBValueList.h"
+#include "lldb/Host/Pipe.h"
+#include "lldb/lldb-types.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Threading.h"
+#include <map>
+#include <mutex>
+#include <optional>
+#include <thread>
+#include <vector>
 
 #define VARREF_LOCALS (int64_t)1
 #define VARREF_GLOBALS (int64_t)2
@@ -138,15 +140,20 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
 
 struct DAP {
   llvm::StringRef debug_adaptor_path;
+  std::optional<std::ofstream> &log;
   InputStream input;
   OutputStream output;
+  lldb::SBFile in;
+  lldb_private::Pipe pout;
+  lldb_private::Pipe perr;
   lldb::SBDebugger debugger;
   lldb::SBTarget target;
   Variables variables;
   lldb::SBBroadcaster broadcaster;
   std::thread event_thread;
   std::thread progress_event_thread;
-  std::unique_ptr<std::ofstream> log;
+  std::thread stdout_forward_thread;
+  std::thread stderr_forward_thread;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   InstructionBreakpointMap instruction_breakpoints;
@@ -198,13 +205,21 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(llvm::StringRef path, ReplMode repl_mode);
+  DAP(llvm::StringRef path, std::optional<std::ofstream> &log,
+      ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output);
   ~DAP();
   DAP(const DAP &rhs) = delete;
   void operator=(const DAP &rhs) = delete;
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
 
+  /// Redirect stdout and stderr fo the IDE's console output.
+  ///
+  /// Errors in this operation will be printed to the log file and the IDE's
+  /// console output as well.
+  llvm::Error ConfigureIO(std::optional<std::FILE *> overrideOut,
+                          std::optional<std::FILE *> overrideErr);
+
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
   void SendJSON(const llvm::json::Value &json);
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index d2e8ec40b0a7b8..a078104595037d 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -87,7 +87,7 @@ bool OutputStream::write_full(llvm::StringRef str) {
   return true;
 }
 
-bool InputStream::read_full(std::ofstream *log, size_t length,
+bool InputStream::read_full(std::optional<std::ofstream> &log, size_t length,
                             std::string &text) {
   std::string data;
   data.resize(length);
@@ -131,7 +131,8 @@ bool InputStream::read_full(std::ofstream *log, size_t length,
   return true;
 }
 
-bool InputStream::read_line(std::ofstream *log, std::string &line) {
+bool InputStream::read_line(std::optional<std::ofstream> &log,
+                            std::string &line) {
   line.clear();
   while (true) {
     if (!read_full(log, 1, line))
@@ -144,7 +145,8 @@ bool InputStream::read_line(std::ofstream *log, std::string &line) {
   return true;
 }
 
-bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
+bool InputStream::read_expected(std::optional<std::ofstream> &log,
+                                llvm::StringRef expected) {
   std::string result;
   if (!read_full(log, expected.size(), result))
     return false;
diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
index 57d5fd458b7165..f4829473b7b0fb 100644
--- a/lldb/tools/lldb-dap/IOStream.h
+++ b/lldb/tools/lldb-dap/IOStream.h
@@ -52,16 +52,24 @@ struct StreamDescriptor {
 struct InputStream {
   StreamDescriptor descriptor;
 
-  bool read_full(std::ofstream *log, size_t length, std::string &text);
+  explicit InputStream(StreamDescriptor descriptor)
+      : descriptor(std::move(descriptor)) {};
 
-  bool read_line(std::ofstream *log, std::string &line);
+  bool read_full(std::optional<std::ofstream> &log, size_t length,
+                 std::string &text);
 
-  bool read_expected(std::ofstream *log, llvm::StringRef expected);
+  bool read_line(std::optional<std::ofstream> &log, std::string &line);
+
+  bool read_expected(std::optional<std::ofstream> &log,
+                     llvm::StringRef expected);
 };
 
 struct OutputStream {
   StreamDescriptor descriptor;
 
+  explicit OutputStream(StreamDescriptor descriptor)
+      : descriptor(std::move(descriptor)) {};
+
   bool write_full(llvm::StringRef str);
 };
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
deleted file mode 100644
index 2c2f49569869b4..00000000000000
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ /dev/null
@@ -1,63 +0,0 @@
-//===-- OutputRedirector.cpp -----------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===/
-
-#if defined(_WIN32)
-#include <fcntl.h>
-#include <io.h>
-#else
-#include <unistd.h>
-#endif
-
-#include "DAP.h"
-#include "OutputRedirector.h"
-#include "llvm/ADT/StringRef.h"
-
-using namespace llvm;
-
-namespace lldb_dap {
-
-Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback) {
-  int new_fd[2];
-#if defined(_WIN32)
-  if (_pipe(new_fd, 4096, O_TEXT) == -1) {
-#else
-  if (pipe(new_fd) == -1) {
-#endif
-    int error = errno;
-    return createStringError(inconvertibleErrorCode(),
-                             "Couldn't create new pipe for fd %d. %s", fd,
-                             strerror(error));
-  }
-
-  if (dup2(new_fd[1], fd) == -1) {
-    int error = errno;
-    return createStringError(inconvertibleErrorCode(),
-                             "Couldn't override the fd %d. %s", fd,
-                             strerror(error));
-  }
-
-  int read_fd = new_fd[0];
-  std::thread t([read_fd, callback]() {
-    char buffer[OutputBufferSize];
-    while (true) {
-      ssize_t bytes_count = read(read_fd, &buffer, sizeof(buffer));
-      if (bytes_count == 0)
-        return;
-      if (bytes_count == -1) {
-        if (errno == EAGAIN || errno == EINTR)
-          continue;
-        break;
-      }
-      callback(StringRef(buffer, bytes_count));
-    }
-  });
-  t.detach();
-  return Error::success();
-}
-
-} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
deleted file mode 100644
index e26d1648b104f9..00000000000000
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ /dev/null
@@ -1,26 +0,0 @@
-//===-- OutputRedirector.h -------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===/
-
-#ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
-#define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
-
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
-
-namespace lldb_dap {
-
-/// Redirects the output of a given file descriptor to a callback.
-///
-/// \return
-///     \a Error::success if the redirection was set up correctly, or an error
-///     otherwise.
-llvm::Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback);
-
-} // namespace lldb_dap
-
-#endif // LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 7e8f7b5f6df679..0e37370f443355 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -10,7 +10,6 @@
 #include "FifoFiles.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
-#include "OutputRedirector.h"
 #include "RunInTerminal.h"
 #include "Watchpoint.h"
 #include "lldb/API/SBDeclaration.h"
@@ -18,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBEvent.h"
 #include "lldb/API/SBStringList.h"
 #include "lldb/Host/Config.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -140,15 +140,14 @@ lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
   }
 }
 
-SOCKET AcceptConnection(DAP &dap, int portno) {
+SOCKET AcceptConnection(std::optional<std::ofstream> &log, int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
   struct sockaddr_in serv_addr, cli_addr;
   SOCKET sockfd = socket(AF_INET, SOCK_STREAM, 0);
   if (sockfd < 0) {
-    if (dap.log)
-      *dap.log << "error: opening socket (" << strerror(errno) << ")"
-               << std::endl;
+    if (log)
+      *log << "error: opening socket (" << strerror(errno) << ")" << std::endl;
   } else {
     memset((char *)&serv_addr, 0, sizeof(serv_addr));
     serv_addr.sin_family = AF_INET;
@@ -156,9 +155,9 @@ SOCKET AcceptConnection(DAP &dap, int portno) {
     serv_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
     serv_addr.sin_port = htons(portno);
     if (bind(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
-      if (dap.log)
-        *dap.log << "error: binding socket (" << strerror(errno) << ")"
-                 << std::endl;
+      if (log)
+        *log << "error: binding socket (" << strerror(errno) << ")"
+             << std::endl;
     } else {
       listen(sockfd, 5);
       socklen_t clilen = sizeof(cli_addr);
@@ -166,8 +165,8 @@ SOCKET AcceptConnection(DAP &dap, int portno) {
           llvm::sys::RetryAfterSignal(static_cast<SOCKET>(-1), accept, sockfd,
                                       (struct sockaddr *)&cli_addr, &clilen);
       if (newsockfd < 0)
-        if (dap.log)
-          *dap.log << "error: accept (" << strerror(errno) << ")" << std::endl;
+        if (log)
+          *log << "error: accept (" << strerror(errno) << ")" << std::endl;
     }
 #if defined(_WIN32)
     closesocket(sockfd);
@@ -1102,6 +1101,14 @@ void request_disconnect(DAP &dap, const llvm::json::Object &request) {
     dap.broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread);
     dap.progress_event_thread.join();
   }
+  if (dap.stdout_forward_thread.joinable()) {
+    dap.pout.Close();
+    dap.stdout_forward_thread.join();
+  }
+  if (dap.stderr_forward_thread.joinable()) {
+    dap.perr.Close();
+    dap.stderr_forward_thread.join();
+  }
   dap.disconnecting = true;
 }
 
@@ -1871,7 +1878,22 @@ void request_initialize(DAP &dap, const llvm::json::Object &request) {
   // which may affect the outcome of tests.
   bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
 
-  dap.debugger = lldb::SBDebugger::Create(source_init_file);
+  // Do not source init files until in/out/err are configured.
+  dap.debugger = lldb::SBDebugger::Create(false);
+  dap.debugger.SetInputFile(dap.in);
+  dap.debugger.SetOutputFile(lldb::SBFile(dap.pout.GetWriteFileDescriptor(), "w", false));
+  dap.debugger.SetErrorFile(lldb::SBFile(dap.perr.GetWriteFileDescriptor(), "w", false));
+
+  auto interp = dap.debugger.GetCommandInterpreter();
+
+  if (source_init_file) {
+    dap.debugger.SkipLLDBInitFiles(false);
+    dap.debugger.SkipAppInitFiles(false);
+    lldb::SBCommandReturnObject init;
+    interp.SourceInitFileInGlobalDirectory(init);
+    interp.SourceInitFileInHomeDirectory(init);
+  }
+
   if (llvm::Error err = dap.RunPreInitCommands()) {
     response["success"] = false;
     EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
@@ -4910,38 +4932,6 @@ static void redirection_test() {
   fflush(stderr);
 }
 
-/// Redirect stdout and stderr fo the IDE's console output.
-///
-/// Errors in this operation will be printed to the log file and the IDE's
-/// console output as well.
-///
-/// \return
-///     A fd pointing to the original stdout.
-static int SetupStdoutStderrRedirection(DAP &dap) {
-  int stdoutfd = fileno(stdout);
-  int new_stdout_fd = dup(stdoutfd);
-  auto output_callback_stderr = [&dap](llvm::StringRef data) {
-    dap.SendOutput(OutputType::Stderr, data);
-  };
-  auto output_callback_stdout = [&dap](llvm::StringRef data) {
-    dap.SendOutput(OutputType::Stdout, data);
-  };
-  if (llvm::Error err = RedirectFd(stdoutfd, output_callback_stdout)) {
-    std::string error_message = llvm::toString(std::move(err));
-    if (dap.log)
-      *dap.log << error_message << std::endl;
-    output_callback_stderr(error_message);
-  }
-  if (llvm::Error err = RedirectFd(fileno(stderr), output_callback_stderr)) {
-    std::string error_message = llvm::toString(std::move(err));
-    if (dap.log)
-      *dap.log << error_message << std::endl;
-    output_callback_stderr(error_message);
-  }
-
-  return new_stdout_fd;
-}
-
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
 #if !defined(__APPLE__)
@@ -5030,6 +5020,11 @@ int main(int argc, char *argv[]) {
   }
 #endif
 
+  std::optional<std::ofstream> log = std::nullopt;
+  const char *log_file_path = getenv("LLDBDAP_LOG");
+  if (log_file_path)
+    log.emplace(log_file_path);
+
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
 
@@ -5037,36 +5032,65 @@ int main(int argc, char *argv[]) {
   auto terminate_debugger =
       llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
 
-  DAP dap = DAP(program_path.str(), default_repl_mode);
-
-  RegisterRequestCallbacks(dap);
-
-  // stdout/stderr redirection to the IDE's console
-  int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+  StreamDescriptor input;
+  StreamDescriptor output;
+  std::optional<std::FILE *> redirectOut = std::nullopt;
+  std::optional<std::FILE *> redirectErr = std::nullopt;
   if (portno != -1) {
     printf("Listening on port %i...\n", portno);
-    SOCKET socket_fd = AcceptConnection(dap, portno);
-    if (socket_fd >= 0) {
-      dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
-      dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
-    } else {
+    SOCKET socket_fd = AcceptConnection(log, portno);
+    if (socket_fd < 0)
       return EXIT_FAILURE;
-    }
+
+    input = StreamDescriptor::from_socket(socket_fd, true);
+    output = StreamDescriptor::from_socket(socket_fd, false);
   } else {
-    dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-    dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+    // Windows opens stdout and stdin in text mode which converts \n to 13,10
+    // while the value is just 10 on Darwin/Linux. Setting the file mode to
+    // binary fixes this.
+    int result = _setmode(fileno(stdout), _O_BINARY);
+    assert(result);
+    result = _setmode(fileno(stdin), _O_BINARY);
+    UNUSED_IF_ASSERT_DISABLED(result);
+    assert(result);
+#endif
 
-    /// used only by TestVSCode_redirection_to_console.py
-    if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
-      redirection_test();
+    int stdout_fd = dup(fileno(stdout));
+    if (stdout_fd == -1) {
+      llvm::errs() << "Failed to configure stdout redirect: "
+                   << lldb_private::Status::FromErrno().takeError() << "\n";
+      return EXIT_FAILURE;
+    }
+
+    redirectOut = stdout;
+    redirectErr = stderr;
+
+    input = StreamDescriptor::from_file(fileno(stdin), false);
+    output = StreamDescriptor::from_file(stdout_fd, false);
   }
 
+  DAP dap = DAP(program_path.str(), log, default_repl_mode, std::move(input),
+                std::move(output));
+
+  // stdout/stderr redirection to the IDE's console
+  if (auto Err = dap.ConfigureIO(redirectOut, redirectErr)) {
+    llvm::errs() << "Failed to configure lldb-dap IO operations: " << Err
+                 << "\n";
+    return EXIT_FAILURE;
+  }
+
+  RegisterRequestCallbacks(dap);
+
   for (const std::string &arg :
        input_args.getAllArgValues(OPT_pre_init_command)) {
     dap.pre_init_commands.push_back(arg);
   }
 
+  // used only by TestVSCode_redirection_to_console.py
+  if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
+    redirection_test();
+
   bool CleanExit = true;
   if (auto Err = dap.Loop()) {
     if (dap.log)

>From b63d2ce4ea1f2677e660624ad0dec404d57b4fd1 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 18 Dec 2024 09:33:50 -0800
Subject: [PATCH 2/5] Applying formatting.

---
 lldb/tools/lldb-dap/DAP.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 4e0de6fa3a4e90..b5c5454a663049 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -11,21 +11,21 @@
 #include "LLDBUtils.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBListener.h"
+#include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Host/FileSystem.h"
-#include "lldb/API/SBCommandReturnObject.h"
-#include "lldb/API/SBProcess.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/Twine.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>

>From 10510203dac68bdfede874a3e15db93e01ec0bd7 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 18 Dec 2024 09:38:34 -0800
Subject: [PATCH 3/5] Applying formatting to lldb-dap.cpp.

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 0e37370f443355..d363eeb115a8a5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -13,11 +13,11 @@
 #include "RunInTerminal.h"
 #include "Watchpoint.h"
 #include "lldb/API/SBDeclaration.h"
+#include "lldb/API/SBEvent.h"
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBStream.h"
-#include "lldb/API/SBEvent.h"
 #include "lldb/API/SBStringList.h"
 #include "lldb/Host/Config.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -1881,8 +1881,10 @@ void request_initialize(DAP &dap, const llvm::json::Object &request) {
   // Do not source init files until in/out/err are configured.
   dap.debugger = lldb::SBDebugger::Create(false);
   dap.debugger.SetInputFile(dap.in);
-  dap.debugger.SetOutputFile(lldb::SBFile(dap.pout.GetWriteFileDescriptor(), "w", false));
-  dap.debugger.SetErrorFile(lldb::SBFile(dap.perr.GetWriteFileDescriptor(), "w", false));
+  dap.debugger.SetOutputFile(
+      lldb::SBFile(dap.pout.GetWriteFileDescriptor(), "w", false));
+  dap.debugger.SetErrorFile(
+      lldb::SBFile(dap.perr.GetWriteFileDescriptor(), "w", false));
 
   auto interp = dap.debugger.GetCommandInterpreter();
 

>From ce969ff36bec0e5796f1961b4e0869336dd7a04a Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 20 Dec 2024 13:40:21 -0800
Subject: [PATCH 4/5] Moving the output redirection logic back into the
 OutputRedirector.cpp file and making it into an object instead.

This removes some of the duplicate code that was happening between stdout/stderr redirection.
---
 lldb/tools/lldb-dap/CMakeLists.txt       |  5 +-
 lldb/tools/lldb-dap/DAP.cpp              | 67 +++++++++------------
 lldb/tools/lldb-dap/DAP.h                | 18 +++---
 lldb/tools/lldb-dap/IOStream.cpp         |  8 +--
 lldb/tools/lldb-dap/IOStream.h           |  8 +--
 lldb/tools/lldb-dap/OutputRedirector.cpp | 77 ++++++++++++++++++++++++
 lldb/tools/lldb-dap/OutputRedirector.h   | 47 +++++++++++++++
 lldb/tools/lldb-dap/lldb-dap.cpp         | 55 ++++++++++++-----
 8 files changed, 212 insertions(+), 73 deletions(-)
 create mode 100644 lldb/tools/lldb-dap/OutputRedirector.cpp
 create mode 100644 lldb/tools/lldb-dap/OutputRedirector.h

diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 00906e8ac10904..43fc18873feb33 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -22,18 +22,19 @@ add_lldb_tool(lldb-dap
   lldb-dap.cpp
   Breakpoint.cpp
   BreakpointBase.cpp
+  DAP.cpp
   ExceptionBreakpoint.cpp
   FifoFiles.cpp
   FunctionBreakpoint.cpp
+  InstructionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
+  OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
-  DAP.cpp
   Watchpoint.cpp
-  InstructionBreakpoint.cpp
 
   LINK_LIBS
     liblldb
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index b5c5454a663049..e99cf8a542fe31 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
+#include "OutputRedirector.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
@@ -49,8 +50,8 @@ using namespace lldb_dap;
 
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, std::optional<std::ofstream> &log,
-         ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output)
+DAP::DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
+         StreamDescriptor input, StreamDescriptor output)
     : debug_adaptor_path(path), log(log), input(std::move(input)),
       output(std::move(output)), broadcaster("lldb-dap"),
       exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
@@ -178,63 +179,55 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
   return nullptr;
 }
 
-llvm::Error DAP::ConfigureIO(std::optional<std::FILE *> overrideOut,
-                             std::optional<std::FILE *> overrideErr) {
+llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
   auto *inull = lldb_private::FileSystem::Instance().Fopen(
       lldb_private::FileSystem::DEV_NULL, "w");
   in = lldb::SBFile(inull, true);
 
-  lldb_private::Status status;
-  status = pout.CreateNew(/*child_process_inherit=*/false);
-  if (status.Fail())
-    return status.takeError();
-  status = perr.CreateNew(/*child_process_inherit=*/false);
-  if (status.Fail())
-    return status.takeError();
+  if (auto Error = out.RedirectTo([this](llvm::StringRef output) {
+        SendOutput(OutputType::Stdout, output);
+      }))
+    return Error;
 
   if (overrideOut) {
-    if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+    auto fd = out.GetWriteFileDescriptor();
+    if (auto Error = fd.takeError())
+      return Error;
+
+    if (dup2(*fd, fileno(overrideOut)) == -1)
       return llvm::make_error<llvm::StringError>(
           llvm::errnoAsErrorCode(),
-          llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+          llvm::formatv("override fd=%d failed", fileno(overrideOut))
               .str()
               .c_str());
-    }
   }
 
+  if (auto Error = err.RedirectTo([this](llvm::StringRef output) {
+        SendOutput(OutputType::Stderr, output);
+      }))
+    return Error;
+
   if (overrideErr) {
-    if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+    auto fd = err.GetWriteFileDescriptor();
+    if (auto Error = fd.takeError())
+      return Error;
+
+    if (dup2(*fd, fileno(overrideErr)) == -1)
       return llvm::make_error<llvm::StringError>(
           llvm::errnoAsErrorCode(),
-          llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+          llvm::formatv("override fd=%d failed", fileno(overrideErr))
               .str()
               .c_str());
-    }
   }
 
-  auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
-    char buffer[4098];
-    size_t bytes_read;
-    while (pipe.CanRead()) {
-      lldb_private::Status error = pipe.ReadWithTimeout(
-          &buffer, sizeof(buffer), std::chrono::seconds(1), bytes_read);
-      if (error.Success()) {
-        // zero bytes returned on EOF.
-        if (bytes_read == 0)
-          break;
-        SendOutput(outputType, llvm::StringRef(buffer, bytes_read));
-      }
-    }
-  };
-
-  stdout_forward_thread =
-      std::thread(forwarder, std::ref(pout), OutputType::Stdout);
-  stderr_forward_thread =
-      std::thread(forwarder, std::ref(perr), OutputType::Stderr);
-
   return llvm::Error::success();
 }
 
+void DAP::StopIO() {
+  out.Stop();
+  err.Stop();
+}
+
 // Send the JSON in "json_str" to the "out" stream. Correctly send the
 // "Content-Length:" field followed by the length, followed by the raw
 // JSON bytes.
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f9cedd32cd0bd3..ae88192e6c8632 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -14,6 +14,7 @@
 #include "FunctionBreakpoint.h"
 #include "IOStream.h"
 #include "InstructionBreakpoint.h"
+#include "OutputRedirector.h"
 #include "ProgressEvent.h"
 #include "SourceBreakpoint.h"
 #include "lldb/API/SBBroadcaster.h"
@@ -27,7 +28,6 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
-#include "lldb/Host/Pipe.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -140,12 +140,12 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
 
 struct DAP {
   llvm::StringRef debug_adaptor_path;
-  std::optional<std::ofstream> &log;
+  std::ofstream *log;
   InputStream input;
   OutputStream output;
   lldb::SBFile in;
-  lldb_private::Pipe pout;
-  lldb_private::Pipe perr;
+  OutputRedirector out;
+  OutputRedirector err;
   lldb::SBDebugger debugger;
   lldb::SBTarget target;
   Variables variables;
@@ -205,8 +205,8 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(llvm::StringRef path, std::optional<std::ofstream> &log,
-      ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output);
+  DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
+      StreamDescriptor input, StreamDescriptor output);
   ~DAP();
   DAP(const DAP &rhs) = delete;
   void operator=(const DAP &rhs) = delete;
@@ -217,8 +217,10 @@ struct DAP {
   ///
   /// Errors in this operation will be printed to the log file and the IDE's
   /// console output as well.
-  llvm::Error ConfigureIO(std::optional<std::FILE *> overrideOut,
-                          std::optional<std::FILE *> overrideErr);
+  llvm::Error ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr);
+
+  /// Stop the redirected IO threads and associated pipes.
+  void StopIO();
 
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index a078104595037d..d2e8ec40b0a7b8 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -87,7 +87,7 @@ bool OutputStream::write_full(llvm::StringRef str) {
   return true;
 }
 
-bool InputStream::read_full(std::optional<std::ofstream> &log, size_t length,
+bool InputStream::read_full(std::ofstream *log, size_t length,
                             std::string &text) {
   std::string data;
   data.resize(length);
@@ -131,8 +131,7 @@ bool InputStream::read_full(std::optional<std::ofstream> &log, size_t length,
   return true;
 }
 
-bool InputStream::read_line(std::optional<std::ofstream> &log,
-                            std::string &line) {
+bool InputStream::read_line(std::ofstream *log, std::string &line) {
   line.clear();
   while (true) {
     if (!read_full(log, 1, line))
@@ -145,8 +144,7 @@ bool InputStream::read_line(std::optional<std::ofstream> &log,
   return true;
 }
 
-bool InputStream::read_expected(std::optional<std::ofstream> &log,
-                                llvm::StringRef expected) {
+bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
   std::string result;
   if (!read_full(log, expected.size(), result))
     return false;
diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
index f4829473b7b0fb..869c5fc8407222 100644
--- a/lldb/tools/lldb-dap/IOStream.h
+++ b/lldb/tools/lldb-dap/IOStream.h
@@ -55,13 +55,11 @@ struct InputStream {
   explicit InputStream(StreamDescriptor descriptor)
       : descriptor(std::move(descriptor)) {};
 
-  bool read_full(std::optional<std::ofstream> &log, size_t length,
-                 std::string &text);
+  bool read_full(std::ofstream *log, size_t length, std::string &text);
 
-  bool read_line(std::optional<std::ofstream> &log, std::string &line);
+  bool read_line(std::ofstream *log, std::string &line);
 
-  bool read_expected(std::optional<std::ofstream> &log,
-                     llvm::StringRef expected);
+  bool read_expected(std::ofstream *log, llvm::StringRef expected);
 };
 
 struct OutputStream {
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
new file mode 100644
index 00000000000000..af4607b57dcf53
--- /dev/null
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -0,0 +1,77 @@
+//===-- OutputRedirector.cpp -----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===/
+
+#include "llvm/Support/Error.h"
+#include <system_error>
+#if defined(_WIN32)
+#include <fcntl.h>
+#include <io.h>
+#else
+#include <unistd.h>
+#endif
+
+#include "DAP.h"
+#include "OutputRedirector.h"
+#include "llvm/ADT/StringRef.h"
+
+using lldb_private::Pipe;
+using lldb_private::Status;
+using llvm::createStringError;
+using llvm::Error;
+using llvm::Expected;
+using llvm::StringRef;
+
+namespace lldb_dap {
+
+Expected<int> OutputRedirector::GetWriteFileDescriptor() {
+  if (!m_pipe.CanWrite())
+    return createStringError(std::errc::bad_file_descriptor,
+                             "write handle is not open for writing");
+  return m_pipe.GetWriteFileDescriptor();
+}
+
+Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
+  Status status = m_pipe.CreateNew(/*child_process_inherit=*/false);
+  if (status.Fail())
+    return status.takeError();
+
+  m_forwarder = std::thread([this, callback]() {
+    char buffer[OutputBufferSize];
+    while (m_pipe.CanRead() && !m_stopped) {
+      size_t bytes_read;
+      Status status = m_pipe.Read(&buffer, sizeof(buffer), bytes_read);
+      if (status.Fail())
+        continue;
+
+      // EOF detected
+      if (bytes_read == 0)
+        break;
+
+      callback(StringRef(buffer, bytes_read));
+    }
+  });
+
+  return Error::success();
+}
+
+void OutputRedirector::Stop() {
+  m_stopped = true;
+
+  if (m_pipe.CanWrite()) {
+    // If the fd is waiting for input and is closed it may not return from the
+    // current select/poll/kqueue/etc. asyncio wait operation. Write a null byte
+    // to ensure the read fd wakes to detect the closed FD.
+    char buf[] = "\0";
+    size_t bytes_written;
+    m_pipe.Write(buf, sizeof(buf), bytes_written);
+    m_pipe.CloseWriteFileDescriptor();
+    m_forwarder.join();
+  }
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
new file mode 100644
index 00000000000000..8bc30ace56719c
--- /dev/null
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -0,0 +1,47 @@
+//===-- OutputRedirector.h -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===/
+
+#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>
+#include <functional>
+#include <thread>
+
+namespace lldb_dap {
+
+struct OutputRedirector {
+  /// Creates writable file descriptor that will invoke the given callback on
+  /// each write in a background thread.
+  ///
+  /// \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::Expected<int> GetWriteFileDescriptor();
+  void Stop();
+
+  ~OutputRedirector() { Stop(); }
+
+  OutputRedirector() = default;
+  OutputRedirector(const OutputRedirector &) = delete;
+  OutputRedirector &operator=(const OutputRedirector &) = delete;
+
+private:
+  std::atomic<bool> m_stopped = false;
+  lldb_private::Pipe m_pipe;
+  std::thread m_forwarder;
+};
+
+} // namespace lldb_dap
+
+#endif // LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d363eeb115a8a5..d8cf1b7d8f0fe4 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -41,6 +41,7 @@
 #include <cassert>
 #include <climits>
 #include <cstdarg>
+#include <cstdint>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
@@ -1101,14 +1102,7 @@ void request_disconnect(DAP &dap, const llvm::json::Object &request) {
     dap.broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread);
     dap.progress_event_thread.join();
   }
-  if (dap.stdout_forward_thread.joinable()) {
-    dap.pout.Close();
-    dap.stdout_forward_thread.join();
-  }
-  if (dap.stderr_forward_thread.joinable()) {
-    dap.perr.Close();
-    dap.stderr_forward_thread.join();
-  }
+  dap.StopIO();
   dap.disconnecting = true;
 }
 
@@ -1881,10 +1875,22 @@ void request_initialize(DAP &dap, const llvm::json::Object &request) {
   // Do not source init files until in/out/err are configured.
   dap.debugger = lldb::SBDebugger::Create(false);
   dap.debugger.SetInputFile(dap.in);
-  dap.debugger.SetOutputFile(
-      lldb::SBFile(dap.pout.GetWriteFileDescriptor(), "w", false));
-  dap.debugger.SetErrorFile(
-      lldb::SBFile(dap.perr.GetWriteFileDescriptor(), "w", false));
+  auto out_fd = dap.out.GetWriteFileDescriptor();
+  if (llvm::Error err = out_fd.takeError()) {
+    response["success"] = false;
+    EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
+    dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+  dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false));
+  auto err_fd = dap.err.GetWriteFileDescriptor();
+  if (llvm::Error err = err_fd.takeError()) {
+    response["success"] = false;
+    EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
+    dap.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+  dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false));
 
   auto interp = dap.debugger.GetCommandInterpreter();
 
@@ -4934,6 +4940,23 @@ static void redirection_test() {
   fflush(stderr);
 }
 
+/// Duplicates a file descriptor, setting FD_CLOEXEC if applicable.
+static int DuplicateFileDescriptor(int fildes) {
+  int fd = ::dup(fildes);
+  if (fd == -1)
+    return fd;
+
+#if defined(FD_CLOEXEC)
+  int flags = ::fcntl(fd, F_GETFD);
+  if (flags == -1)
+    return -1;
+  if (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) != 0)
+    return -1;
+#else
+  return fd;
+#endif
+}
+
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
 #if !defined(__APPLE__)
@@ -5036,8 +5059,8 @@ int main(int argc, char *argv[]) {
 
   StreamDescriptor input;
   StreamDescriptor output;
-  std::optional<std::FILE *> redirectOut = std::nullopt;
-  std::optional<std::FILE *> redirectErr = std::nullopt;
+  std::FILE *redirectOut = nullptr;
+  std::FILE *redirectErr = nullptr;
   if (portno != -1) {
     printf("Listening on port %i...\n", portno);
     SOCKET socket_fd = AcceptConnection(log, portno);
@@ -5058,7 +5081,7 @@ int main(int argc, char *argv[]) {
     assert(result);
 #endif
 
-    int stdout_fd = dup(fileno(stdout));
+    int stdout_fd = DuplicateFileDescriptor(fileno(stdout));
     if (stdout_fd == -1) {
       llvm::errs() << "Failed to configure stdout redirect: "
                    << lldb_private::Status::FromErrno().takeError() << "\n";
@@ -5072,7 +5095,7 @@ int main(int argc, char *argv[]) {
     output = StreamDescriptor::from_file(stdout_fd, false);
   }
 
-  DAP dap = DAP(program_path.str(), log, default_repl_mode, std::move(input),
+  DAP dap = DAP(program_path.str(), &*log, default_repl_mode, std::move(input),
                 std::move(output));
 
   // stdout/stderr redirection to the IDE's console

>From 4112837b2925c35d89543aeb115969abd7c394ea Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 20 Dec 2024 14:24:54 -0800
Subject: [PATCH 5/5] Simplifying how the log object is created, using a
 unique_ptr instead of an optional.

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index d8cf1b7d8f0fe4..3ca17884ec5166 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -141,7 +141,7 @@ lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
   }
 }
 
-SOCKET AcceptConnection(std::optional<std::ofstream> &log, int portno) {
+SOCKET AcceptConnection(std::ofstream *log, int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
   struct sockaddr_in serv_addr, cli_addr;
@@ -5045,10 +5045,10 @@ int main(int argc, char *argv[]) {
   }
 #endif
 
-  std::optional<std::ofstream> log = std::nullopt;
+  std::unique_ptr<std::ofstream> log = nullptr;
   const char *log_file_path = getenv("LLDBDAP_LOG");
   if (log_file_path)
-    log.emplace(log_file_path);
+    log.reset(new std::ofstream(log_file_path));
 
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
@@ -5063,7 +5063,7 @@ int main(int argc, char *argv[]) {
   std::FILE *redirectErr = nullptr;
   if (portno != -1) {
     printf("Listening on port %i...\n", portno);
-    SOCKET socket_fd = AcceptConnection(log, portno);
+    SOCKET socket_fd = AcceptConnection(log.get(), portno);
     if (socket_fd < 0)
       return EXIT_FAILURE;
 
@@ -5095,8 +5095,8 @@ int main(int argc, char *argv[]) {
     output = StreamDescriptor::from_file(stdout_fd, false);
   }
 
-  DAP dap = DAP(program_path.str(), &*log, default_repl_mode, std::move(input),
-                std::move(output));
+  DAP dap = DAP(program_path.str(), log.get(), default_repl_mode,
+                std::move(input), std::move(output));
 
   // stdout/stderr redirection to the IDE's console
   if (auto Err = dap.ConfigureIO(redirectOut, redirectErr)) {
@@ -5118,8 +5118,8 @@ int main(int argc, char *argv[]) {
 
   bool CleanExit = true;
   if (auto Err = dap.Loop()) {
-    if (dap.log)
-      *dap.log << "Transport Error: " << llvm::toString(std::move(Err)) << "\n";
+    if (log)
+      *log << "Transport Error: " << llvm::toString(std::move(Err)) << "\n";
     CleanExit = false;
   }
 



More information about the lldb-commits mailing list