[Lldb-commits] [lldb] [lldb] Keep the inferior's terminal open until LLDB read its output (PR #229387)
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 6 04:44:34 PDT 2026
https://github.com/Teemperor created https://github.com/llvm/llvm-project/pull/229387
lldb gives the inferior the secondary side of a pseudo terminal for its stdio and drains the primary side (what we get back from from posix_openpt) via ThreadedCommunication's read thread.
On Darwin, it seems that closing the last secondary descriptor clears the terminal's contents. Any output the inferior wrote but that the read thread has not consumed yet is then destroyed alongside the inferior.
There isn't really any guarantee that we read the inferior's output before it dies (and closes its secondary side of the terminal). So reading stdout is currently racy in LLDB. This is also why TestTargetAPI.py randomly fails to read the output:
```
File ".../lldb/test/API/python_api/target/TestTargetAPI.py", line 189
self.assertIn("arg: foo", output)
AssertionError: 'arg: foo' not found in ''
```
The easiest way to make this race more likely is to sleep at the start of ThreadedCommunication::ReadThread's main loop like this:
```
static std::chrono::milliseconds g_stdio_read_delay(800);
// in ThreadedCommunication::ReadThrea
while (!done && m_read_thread_enabled) {
// sleep if we're the stdio reader thread.
if (GetBroadcasterName() == "process.stdio")
std::this_thread::sleep_for(g_stdio_read_delay);
```
This patch fixes this by also making LLDB open the secondary side of the terminal so that the the pty outlives the inferior. Then, LLDB can read all stdout contents and close the secondary (and the pty).
assisted-by: claude
>From b3c5f5503561594d962517eba93834d011c58034 Mon Sep 17 00:00:00 2001
From: Raphael Isemann <rise at apple.com>
Date: Thu, 17 Sep 2026 15:12:17 +0100
Subject: [PATCH] [lldb] Keep the inferior's terminal open until LLDB read its
output
lldb gives the inferior the secondary side of a pseudo terminal for its
stdio and drains the primary side (what we get back from from
posix_openpt) via ThreadedCommunication's read thread.
On Darwin, it seems that closing the last secondary descriptor clears
the terminal's contents. Any output the inferior wrote but that the
read thread has not consumed yet is then destroyed alongside the
inferior.
There isn't really any guarantee that we read the inferior's output
before it dies (and closes its secondary side of the terminal). So
reading stdout is currently racy in LLDB. This is also why
TestTargetAPI.py randomly fails to read the output:
```
File ".../lldb/test/API/python_api/target/TestTargetAPI.py", line 189
self.assertIn("arg: foo", output)
AssertionError: 'arg: foo' not found in ''
```
The easiest way to make this race more likely is to sleep at the
start of ThreadedCommunication::ReadThread's main loop like this:
```
static std::chrono::milliseconds g_stdio_read_delay(800);
// in ThreadedCommunication::ReadThrea
while (!done && m_read_thread_enabled) {
// sleep if we're the stdio reader thread.
if (GetBroadcasterName() == "process.stdio")
std::this_thread::sleep_for(g_stdio_read_delay);
```
This patch fixes this by also making LLDB open the secondary side of the
terminal so that the the pty outlives the inferior. Then, LLDB can read
all stdout contents and close the secondary (and the pty).
assisted-by: claude
---
lldb/include/lldb/Target/Process.h | 32 +++++++++-
.../MacOSX/PlatformAppleSimulator.cpp | 6 +-
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 6 +-
.../Platform/QemuUser/PlatformQemuUser.cpp | 3 +-
.../Platform/WebAssembly/PlatformWasm.cpp | 3 +-
.../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +-
lldb/source/Target/Platform.cpp | 6 +-
lldb/source/Target/Process.cpp | 63 ++++++++++++++++---
8 files changed, 98 insertions(+), 23 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 984a4ef880800..231f8dd1d79d6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -53,6 +53,7 @@
#include "lldb/Utility/Broadcaster.h"
#include "lldb/Utility/Event.h"
#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Locked.h"
#include "lldb/Utility/NameMatches.h"
#include "lldb/Utility/Policy.h"
#include "lldb/Utility/ProcessAddress.h"
@@ -2663,10 +2664,27 @@ void PruneThreadPlans();
/// assumed to be valid and will be managed by the newly created
/// connection.
///
+ /// \param[in] secondary_fd A descriptor for the secondary side of that same
+ /// terminal as file_descriptor. This is used to keep the terminal (and
+ /// its output buffer alive). The process takes ownership of it and keeps
+ /// it open until it stops monitoring fd.
+ ///
+ /// Pass std::nullopt if fd is not a pseudo terminal or no such
+ /// descriptor is available.
+ ///
/// \see lldb_private::Process::STDIOReadThreadBytesReceived()
/// \see lldb_private::IOHandlerProcessSTDIO
/// \see lldb_private::ConnectionFileDescriptor
- void SetSTDIOFileDescriptor(int file_descriptor);
+ void SetSTDIOFileDescriptor(int file_descriptor,
+ std::optional<int> secondary_fd = std::nullopt);
+
+#if !defined(_WIN32)
+ /// Associates the primary side of \a pty with the process' STDIO handling.
+ /// The process takes ownership of both of \a pty's descriptors.
+ ///
+ /// \see SetSTDIOFileDescriptor()
+ void SetSTDIOPseudoTerminal(PseudoTerminal &pty);
+#endif
// Add a permanent region of memory that should never be read or written to.
// This can be used to ensure that memory reads or writes to certain areas of
@@ -3520,6 +3538,9 @@ void PruneThreadPlans();
mutable std::mutex m_process_input_reader_mutex;
ThreadedCommunication m_stdio_communication;
std::recursive_mutex m_stdio_communication_mutex;
+ /// The secondary side of the pseudo terminal the inferior uses for stdio.
+ /// std::nullopt if no pseudo terminal is open.
+ Guarded<std::optional<int>, std::mutex> m_stdio_secondary_fd;
bool m_stdin_forward; /// Remember if stdin must be forwarded to remote debug
/// server
std::string m_stdout_data;
@@ -3680,6 +3701,15 @@ void PruneThreadPlans();
static void STDIOReadThreadBytesReceived(void *baton, const void *src,
size_t src_len);
+ /// Stop monitoring the process' stdio.
+ ///
+ /// \param drain
+ /// Whether to read any remaining stdio buffers before.
+ void StopSTDIOMonitoring(bool drain);
+
+ /// Close m_stdio_secondary_fd if it is still open.
+ void CloseSTDIOSecondaryFileDescriptor();
+
bool PushProcessIOHandler();
bool PopProcessIOHandler();
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 6a04a9a3840cc..fb8784ed8919c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -215,9 +215,9 @@ PlatformAppleSimulator::DebugProcess(ProcessLaunchInfo &launch_info,
// been used where the secondary side was given as the file to open for
// stdin/out/err after we have already opened the primary so we can
// read/write stdin/out/err.
- int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
- if (pty_fd != PseudoTerminal::invalid_fd) {
- process_sp->SetSTDIOFileDescriptor(pty_fd);
+ if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
+ PseudoTerminal::invalid_fd) {
+ process_sp->SetSTDIOPseudoTerminal(launch_info.GetPTY());
}
}
}
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 2a5a3d55ec98c..4b3a3a4cf16ce 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -493,9 +493,9 @@ lldb::ProcessSP PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info,
// Hook up process PTY if we have one (which we should for local debugging
// with llgs).
#ifndef _WIN32 // TODO: Implement on Windows
- int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
- if (pty_fd != PseudoTerminal::invalid_fd) {
- process_sp->SetSTDIOFileDescriptor(pty_fd);
+ if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
+ PseudoTerminal::invalid_fd) {
+ process_sp->SetSTDIOPseudoTerminal(launch_info.GetPTY());
LLDB_LOG(log, "hooked up STDIO pty to process");
} else
LLDB_LOG(log, "not using process STDIO pty");
diff --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index ea446f67e0fca..cf6450a1ba171 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -238,8 +238,7 @@ lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
#ifndef _WIN32 // TODO: Implement on Windows
if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
PseudoTerminal::invalid_fd)
- process_sp->SetSTDIOFileDescriptor(
- launch_info.GetPTY().ReleasePrimaryFileDescriptor());
+ process_sp->SetSTDIOPseudoTerminal(launch_info.GetPTY());
#endif
return process_sp;
diff --git a/lldb/source/Plugins/Platform/WebAssembly/PlatformWasm.cpp b/lldb/source/Plugins/Platform/WebAssembly/PlatformWasm.cpp
index 2dee06e414c5b..8259a63f70dea 100644
--- a/lldb/source/Plugins/Platform/WebAssembly/PlatformWasm.cpp
+++ b/lldb/source/Plugins/Platform/WebAssembly/PlatformWasm.cpp
@@ -266,8 +266,7 @@ lldb::ProcessSP PlatformWasm::DebugProcess(ProcessLaunchInfo &launch_info,
#ifndef _WIN32
if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
PseudoTerminal::invalid_fd)
- process_sp->SetSTDIOFileDescriptor(
- launch_info.GetPTY().ReleasePrimaryFileDescriptor());
+ process_sp->SetSTDIOPseudoTerminal(launch_info.GetPTY());
#endif
return process_sp;
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 151932b67dca9..af081c1fd168a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -945,7 +945,7 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module,
if (!disable_stdio) {
if (pty.GetPrimaryFileDescriptor() != PseudoTerminal::invalid_fd) {
- SetSTDIOFileDescriptor(pty.ReleasePrimaryFileDescriptor());
+ SetSTDIOPseudoTerminal(pty);
}
#ifdef _WIN32
else if (m_stdin_forward) {
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 687179a02aaea..919d8e7bc035a 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1153,9 +1153,9 @@ lldb::ProcessSP Platform::DebugProcess(ProcessLaunchInfo &launch_info,
// stdin/out/err after we have already opened the primary so we can
// read/write stdin/out/err.
#ifndef _WIN32
- int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
- if (pty_fd != PseudoTerminal::invalid_fd) {
- process_sp->SetSTDIOFileDescriptor(pty_fd);
+ if (launch_info.GetPTY().GetPrimaryFileDescriptor() !=
+ PseudoTerminal::invalid_fd) {
+ process_sp->SetSTDIOPseudoTerminal(launch_info.GetPTY());
}
#endif
} else {
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index c86160ae0432d..a022ca76b42db 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -12,6 +12,7 @@
#include <optional>
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/Threading.h"
@@ -579,6 +580,9 @@ Process::~Process() {
LLDB_LOGF(log, "%p Process::~Process()", static_cast<void *>(this));
StopPrivateStateThread();
+ // Close the secondary to make sure the pty is not retained.
+ CloseSTDIOSecondaryFileDescriptor();
+
// ThreadList::Clear() will try to acquire this process's mutex, so
// explicitly clear the thread list here to ensure that the mutex is not
// destroyed before the thread list.
@@ -3901,9 +3905,9 @@ Status Process::DestroyImpl(bool force_kill) {
DidDestroy();
StopPrivateStateThread();
}
- m_stdio_communication.StopReadThread();
- m_stdio_communication.Disconnect();
- m_stdin_forward = false;
+ // Not draining: We are tearing the process down, so we don't care about its
+ // remaining output as no one could read it.
+ StopSTDIOMonitoring(/*drain=*/false);
{
std::lock_guard<std::mutex> guard(m_process_input_reader_mutex);
@@ -3972,10 +3976,9 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) {
case eStateDetached:
case eStateExited:
case eStateUnloaded:
- m_stdio_communication.SynchronizeWithReadThread();
- m_stdio_communication.StopReadThread();
- m_stdio_communication.Disconnect();
- m_stdin_forward = false;
+ // The inferior is gone, so this is our last chance to pick up whatever it
+ // wrote before it went away.
+ StopSTDIOMonitoring(/*drain=*/true);
[[fallthrough]];
case eStateConnected:
@@ -5007,7 +5010,13 @@ void Process::STDIOReadThreadBytesReceived(void *baton, const void *src,
process->AppendSTDOUT(static_cast<const char *>(src), src_len);
}
-void Process::SetSTDIOFileDescriptor(int fd) {
+void Process::SetSTDIOFileDescriptor(int fd, std::optional<int> secondary_fd) {
+ {
+ auto stdio_secondary_fd = m_stdio_secondary_fd.Lock();
+ assert(!*stdio_secondary_fd && "process stdio is already set up");
+ *stdio_secondary_fd = secondary_fd;
+ }
+
// First set up the Read Thread for reading/handling process I/O
m_stdio_communication.SetConnection(
std::make_unique<ConnectionFileDescriptor>(fd, true));
@@ -5023,7 +5032,45 @@ void Process::SetSTDIOFileDescriptor(int fd) {
m_process_input_reader =
std::make_shared<IOHandlerProcessSTDIO>(this, fd);
}
+ } else {
+ // Nobody is going to read the primary side, so there is no point in keeping
+ // the terminal alive.
+ CloseSTDIOSecondaryFileDescriptor();
+ }
+}
+
+#if !defined(_WIN32)
+void Process::SetSTDIOPseudoTerminal(PseudoTerminal &pty) {
+ // Take a descriptor for the secondary side so it is kept alive.
+ if (llvm::Error err = pty.OpenSecondary(O_RDWR | O_NOCTTY | O_CLOEXEC))
+ LLDB_LOG_ERROR(GetLog(LLDBLog::Process), std::move(err), "failed to open the
+ secondary side of the inferior's " "terminal: {0}");
+
+ std::optional<int> secondary_fd;
+ if (int fd = pty.ReleaseSecondaryFileDescriptor();
+ fd != PseudoTerminal::invalid_fd)
+ secondary_fd = fd;
+ SetSTDIOFileDescriptor(pty.ReleasePrimaryFileDescriptor(), secondary_fd);
+}
+#endif
+
+void Process::StopSTDIOMonitoring(bool drain) {
+ if (drain) {
+ // Collect everything the inferior has written.
+ m_stdio_communication.SynchronizeWithReadThread();
}
+
+ // Nothing is left to read, so let go of the terminal.
+ CloseSTDIOSecondaryFileDescriptor();
+
+ m_stdio_communication.StopReadThread();
+ m_stdio_communication.Disconnect();
+ m_stdin_forward = false;
+}
+
+void Process::CloseSTDIOSecondaryFileDescriptor() {
+ if (std::optional<int> fd = std::exchange(*m_stdio_secondary_fd.Lock(), {}))
+ llvm::sys::Process::SafelyCloseFileDescriptor(*fd);
}
bool Process::ProcessIOHandlerIsActive() {
More information about the lldb-commits
mailing list