[Lldb-commits] [lldb] [lldb-dap] Use MainLoop instead of a background thread in OutputRedirector. (PR #199970)
via lldb-commits
lldb-commits at lists.llvm.org
Wed May 27 06:46:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Ebuka Ezike (da-viper)
<details>
<summary>Changes</summary>
Replace the background thread in OutputRedirector with LLDB's MainLoop event loop. This reduces the number of threads created and ensures file descriptors are properly closed when no longer needed.
Since debugger's output is not I/O intensive, there is no risk of hitting the pipe buffer limit with this approach.
---
Full diff: https://github.com/llvm/llvm-project/pull/199970.diff
3 Files Affected:
- (modified) lldb/tools/lldb-dap/DAP.cpp (+14-11)
- (modified) lldb/tools/lldb-dap/OutputRedirector.cpp (+40-35)
- (modified) lldb/tools/lldb-dap/OutputRedirector.h (+23-12)
``````````diff
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index c52c0ee898eb8..a1bf829b646a3 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -227,15 +227,21 @@ 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(overrideOut, [this](llvm::StringRef output) {
- SendOutput(OutputType::Console, output);
- }))
- return Error;
+ if (auto error = out.RedirectTo(
+ m_loop, overrideOut,
+ [this](llvm::StringRef output) {
+ SendOutput(OutputType::Console, output);
+ },
+ log))
+ return error;
- if (auto Error = err.RedirectTo(overrideErr, [this](llvm::StringRef output) {
- SendOutput(OutputType::Console, output);
- }))
- return Error;
+ if (auto error = err.RedirectTo(
+ m_loop, overrideErr,
+ [this](llvm::StringRef output) {
+ SendOutput(OutputType::Console, output);
+ },
+ log))
+ return error;
return llvm::Error::success();
}
@@ -1044,9 +1050,6 @@ llvm::Error DAP::Loop() {
auto thread = std::thread([this] { TransportHandler(); });
llvm::scope_exit cleanup([this]() {
- // FIXME: Merge these into the MainLoop handler.
- out.Stop();
- err.Stop();
StopEventHandlers();
// Destroy the debugger when the session ends. This will trigger the
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
index fe278faca87bf..b5213769fde79 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -8,6 +8,9 @@
#include "OutputRedirector.h"
#include "DAP.h"
+#include "DAPLog.h"
+#include "lldb/Host/File.h"
+#include "lldb/Host/MainLoopBase.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <cstring>
@@ -20,8 +23,7 @@
#endif
using namespace llvm;
-
-static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0");
+using namespace lldb_private;
namespace lldb_dap {
@@ -38,9 +40,10 @@ Expected<int> OutputRedirector::GetWriteFileDescriptor() {
return m_fd;
}
-Error OutputRedirector::RedirectTo(std::FILE *file_override,
- std::function<void(StringRef)> callback) {
- assert(m_fd == kInvalidDescriptor && "Output readirector already started.");
+Error OutputRedirector::RedirectTo(MainLoopBase &loop, std::FILE *file_override,
+ std::function<void(StringRef)> callback,
+ Log &log) {
+ assert(m_fd == kInvalidDescriptor && "OutputRedirector already started.");
int new_fd[2];
#if defined(_WIN32)
@@ -56,6 +59,9 @@ Error OutputRedirector::RedirectTo(std::FILE *file_override,
int read_fd = new_fd[0];
m_fd = new_fd[1];
+ m_read_obj = std::make_shared<NativeFile>(read_fd, File::eOpenOptionReadOnly,
+ NativeFile::Owned);
+
if (file_override) {
int override_fd = fileno(file_override);
@@ -68,46 +74,44 @@ Error OutputRedirector::RedirectTo(std::FILE *file_override,
return llvm::errorCodeToError(llvm::errnoAsErrorCode());
}
- m_forwarder = std::thread([this, callback, read_fd]() {
- char buffer[OutputBufferSize];
- while (!m_stopped) {
- ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
- if (bytes_count == -1) {
- // Skip non-fatal errors.
- if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK)
- continue;
- break;
- }
- // Skip the null byte used to trigger a Stop.
- if (bytes_count == 1 && buffer[0] == '\0')
- continue;
-
- StringRef data(buffer, bytes_count);
- if (m_stopped)
- data.consume_back(kCloseSentinel);
- if (data.empty())
- break;
-
- callback(data);
+ auto read_callback = [callback = std::move(callback), this,
+ &log](MainLoopBase &) {
+ std::array<char, OutputBufferSize> buffer;
+ size_t num_bytes = buffer.size();
+
+ const Status status = m_read_obj->Read(buffer.data(), num_bytes);
+ if (status.Fail()) {
+ DAP_LOG_ERROR(log, status.ToError(),
+ "OutputRedirector read failed (handle {1}): error: {0}",
+ m_read_obj->GetWaitableHandle());
+ m_read_handle.reset();
+ return;
+ }
+ if (num_bytes == 0) { // EOF
+ m_read_handle.reset();
+ return;
}
- ::close(read_fd);
- });
- return Error::success();
+ const llvm::StringRef data(buffer.data(), num_bytes);
+ callback(data);
+ };
+
+ Status status;
+ m_read_handle =
+ loop.RegisterReadObject(m_read_obj, std::move(read_callback), status);
+
+ return status.takeError();
}
void OutputRedirector::Stop() {
- m_stopped = true;
+ // Stop polling.
+ m_read_handle.reset();
+ m_read_obj.reset();
if (m_fd != kInvalidDescriptor) {
int fd = m_fd;
m_fd = kInvalidDescriptor;
- // 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.data(), kCloseSentinel.size());
::close(fd);
- m_forwarder.join();
// Restore the fd back to its original state since we stopped the
// redirection.
@@ -118,6 +122,7 @@ void OutputRedirector::Stop() {
int original_fd = m_original_fd;
m_original_fd = kInvalidDescriptor;
::dup2(restore_fd, original_fd);
+ ::close(restore_fd);
}
}
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
index 77b1b76ec4d89..959375debc828 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -9,35 +9,44 @@
#ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
#define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
+#include "lldb/Host/MainLoop.h"
+#include "lldb/Host/MainLoopBase.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
-#include <atomic>
#include <functional>
-#include <thread>
namespace lldb_dap {
-
+class Log;
class OutputRedirector {
public:
static int kInvalidDescriptor;
- /// Creates writable file descriptor that will invoke the given callback on
- /// each write in a background thread.
+ /// Creates a writable file descriptor that will invoke the given callback
+ /// when data is written, dispatched from the provided main loop.
+ ///
+ /// \param[in] loop
+ /// The main loop used to poll the read end of the redirection pipe.
///
/// \param[in] file_override
- /// Updates the file descriptor to the redirection pipe, if not null.
+ /// If non-null, redirects this file's descriptor to the pipe so writes
+ /// to it are captured.
///
/// \param[in] callback
- /// A callback invoked when any data is written to the file handle.
+ /// Invoked on the main loop thread with each chunk of data read from
+ /// the pipe.
+ ///
+ /// \param[in] log
+ /// Used to report read errors from the redirection pipe.
///
/// \return
/// \a Error::success if the redirection was set up correctly, or an error
/// otherwise.
- llvm::Error RedirectTo(std::FILE *file_override,
- std::function<void(llvm::StringRef)> callback);
+ llvm::Error RedirectTo(lldb_private::MainLoopBase &loop,
+ std::FILE *file_override,
+ std::function<void(llvm::StringRef)> callback,
+ Log &log);
llvm::Expected<int> GetWriteFileDescriptor();
- void Stop();
~OutputRedirector() { Stop(); }
@@ -46,11 +55,13 @@ class OutputRedirector {
OutputRedirector &operator=(const OutputRedirector &) = delete;
private:
- std::atomic<bool> m_stopped = false;
+ void Stop();
+
int m_fd;
int m_original_fd;
int m_restore_fd;
- std::thread m_forwarder;
+ lldb::IOObjectSP m_read_obj;
+ lldb_private::MainLoop::ReadHandleUP m_read_handle;
};
} // namespace lldb_dap
``````````
</details>
https://github.com/llvm/llvm-project/pull/199970
More information about the lldb-commits
mailing list