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

[Pavel Labath via lldb-commits]

================
@@ -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);
----------------
labath wrote:

If this really can be a read-until-EOF loop, then you could just make this a blocking read (no timeout). *However*, I think this is not a good idea to do in combination with duping the pipe to stdout/err (TIL that `dup2` is a thing on windows), because in order to get an EOF you have to close *all* copies of that FD. It looks like you don't do that, which is probably the cause of the presubmit test failures, but even if you did (perhaps by restoring the original FD, or /dev/null), I still wouldn't really like this approach because it's very easy for stdout/err descriptors to be leaked into other processes (which might end up outliving all lldb-dap connections).

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