[lldb] Serialize scripted-command output with the statusline (#208609)

A Python command's print() writes to sys.stdout, which the interpreter
session pointed at the debugger's raw terminal descriptor via
PyFile_FromFd. Those writes bypass the debugger's output lock, so they
can race with the statusline, resulting in truncated output. Back the
session's terminal stdout and stderr with a pipe instead. A reader
thread drains the pipe and writes to the terminal through
Debugger::PrintAsync, which takes the same output lock as the
statusline.

Only redirect when a statusline could actually be drawing: the pipe and
reader thread are set up only when StatuslineSupported() holds
(show-statusline is enabled and the output is an escape-code-capable
terminal) and the target is the debugger's own terminal. The interactive
interpreter opts out entirely, since input()'s readline line editing and
echo need both sys.stdin and sys.stdout to be the real terminal.

Python still gets a real line-buffered text file over the pipe, so
output stays live and file objects passed to SB APIs
(SetImmediateOutputFile, subprocess, fileno) keep working. On session
teardown the wrappers are flushed before the pipe is closed so a
retained sys.stdout does not strand a trailing line or write into a
closed descriptor.

rdar://181841574

lldb/include/lldb/Core/Debugger.h

@@ -458,6 +458,10 @@ public:
   /// Redraw the statusline if enabled.
   void RedrawStatusline(std::optional<ExecutionContextRef> exe_ctx_ref);
 
+  /// Whether the statusline can be drawn: show-statusline is enabled and the
+  /// output is an escape-code-capable terminal.
+  bool StatuslineSupported();
+
   /// Flush cached state (e.g. stale execution context in the statusline).
   void FlushStatusLine();
 
@@ -717,7 +721,6 @@ protected:
   /// @}
 
   bool IsEscapeCodeCapableTTY();
-  bool StatuslineSupported();
 
   void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
                      bool cancel_top_handler = true);

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -25,6 +25,7 @@
 #include "lldb/Core/ThreadedCommunication.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/Host/Config.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/Pipe.h"
@@ -46,6 +47,10 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatAdapters.h"
 
+#if defined(_WIN32)
+#include "lldb/Host/windows/ConnectionGenericFileWindows.h"
+#endif
+
 #include <cstdio>
 #include <cstdlib>
 #include <memory>
@@ -444,6 +449,88 @@ ScriptInterpreterPythonImpl::ScriptInterpreterPythonImpl(Debugger &debugger)
   RunSimpleString(run_string.GetData());
 }
 
+/// A Python sys.stdout/stderr file backed by a pipe whose read end is drained
+/// by a reader thread that writes to the debugger's terminal under the output
+/// lock (Debugger::PrintAsync). Handing Python the raw terminal descriptor
+/// instead lets a script's print() race the statusline, which redraws on the
+/// event thread under that lock. Its cursor save/restore then rewinds over and
+/// eats the script's output.
+class ScriptInterpreterPythonImpl::SessionIORedirect {
+public:
+  static std::unique_ptr<SessionIORedirect> Create(lldb::user_id_t debugger_id,
+                                                   bool is_stdout) {
+    Pipe pipe;
+    if (pipe.CreateNew().Fail())
+      return nullptr;
+
+    std::unique_ptr<SessionIORedirect> redirect(
+        new SessionIORedirect(debugger_id, is_stdout));
+
+#if defined(_WIN32)
+    lldb::file_t read_handle = pipe.GetReadNativeHandle();
+    pipe.ReleaseReadFileDescriptor();
+    std::unique_ptr<Connection> conn =
+        std::make_unique<ConnectionGenericFile>(read_handle, true);
+#else
+    std::unique_ptr<Connection> conn =
+        std::make_unique<ConnectionFileDescriptor>(
+            pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true);
+#endif
+    if (!conn->IsConnected())
+      return nullptr;
+
+    redirect->m_communication.SetConnection(std::move(conn));
+    redirect->m_communication.SetReadThreadBytesReceivedCallback(
+        ReadThreadBytesReceived, redirect.get());
+    if (!redirect->m_communication.StartReadThread())
+      return nullptr;
+    redirect->m_connected = true;
+
+    // The write end is owned here. Python only borrows its descriptor.
+    redirect->m_write_file_sp = std::make_shared<NativeFile>(
+        pipe.ReleaseWriteFileDescriptor(), File::eOpenOptionWriteOnly,
+        NativeFile::Owned);
+    return redirect;
+  }
+
+  ~SessionIORedirect() {
+    if (!m_connected)
+      return;
+    // Close the write end so the reader sees EOF and exits, then join it.
+    if (m_write_file_sp)
+      m_write_file_sp->Close();
+    m_communication.JoinReadThread();
+    m_communication.Disconnect();
+  }
+
+  int GetWriteDescriptor() const {
+    return m_write_file_sp ? m_write_file_sp->GetDescriptor()
+                           : File::kInvalidDescriptor;
+  }
+
+private:
+  SessionIORedirect(lldb::user_id_t debugger_id, bool is_stdout)
+      : m_debugger_id(debugger_id), m_is_stdout(is_stdout),
+        m_communication("lldb.ScriptInterpreterPython.io-redirect") {}
+
+  static void ReadThreadBytesReceived(void *baton, const void *src,
+                                      size_t src_len) {
+    if (!src || !src_len)
+      return;
+    auto *self = static_cast<SessionIORedirect *>(baton);
+    if (lldb::DebuggerSP debugger_sp =
+            Debugger::FindDebuggerWithID(self->m_debugger_id))
+      debugger_sp->PrintAsync(static_cast<const char *>(src), src_len,
+                              self->m_is_stdout);
+  }
+
+  lldb::user_id_t m_debugger_id;
+  bool m_is_stdout;
+  lldb::FileSP m_write_file_sp;
+  ThreadedCommunication m_communication;
+  bool m_connected = false;
+};
+
 ScriptInterpreterPythonImpl::~ScriptInterpreterPythonImpl() {
   // the session dictionary may hold objects with complex state which means
   // that they may need to be torn down with some level of smarts and that, in
@@ -567,6 +654,26 @@ void ScriptInterpreterPythonImpl::LeaveSession() {
   if (PyThreadState_GetDict()) {
     PythonDictionary &sys_module_dict = GetSysModuleDictionary();
     if (sys_module_dict.IsValid()) {
+      // Flush the pipe-backed wrappers while they are still sys.stdout/stderr.
+      // Line buffering already flushes on each newline, but a trailing
+      // unterminated line would otherwise be stranded (and later flushed into
+      // a closed descriptor) once we close the pipe write end below.
+      auto flush_redirect = [&](const char *py_name,
+                                std::unique_ptr<SessionIORedirect> &redirect) {
+        if (!redirect)
+          return;
+        PythonObject file =
+            sys_module_dict.GetItemForKey(PythonString(py_name));
+        if (!file.IsValid())
+          return;
+        if (llvm::Expected<PythonObject> result = file.CallMethod("flush"))
+          (void)result;
+        else
+          llvm::consumeError(result.takeError());
+      };
+      flush_redirect("stdout", m_stdout_redirect);
+      flush_redirect("stderr", m_stderr_redirect);
+
       if (m_saved_stdin.IsValid()) {
         sys_module_dict.SetItemForKey(PythonString("stdin"), m_saved_stdin);
         m_saved_stdin.Reset();
@@ -582,19 +689,86 @@ void ScriptInterpreterPythonImpl::LeaveSession() {
     }
   }
 
+  // Tear down the pipe redirects (closes each write end and joins its reader).
+  // The wrappers were flushed above, so nothing buffered is lost.
+  m_stdout_redirect.reset();
+  m_stderr_redirect.reset();
+
   m_session_is_active = false;
 }
 
+bool ScriptInterpreterPythonImpl::RedirectTerminalHandleThroughLock(
+    const char *py_name, PythonObject &save_file, const char *mode,
+    File &file) {
+  const bool is_stdout = ::strcmp(py_name, "stdout") == 0;
+  if (!is_stdout && ::strcmp(py_name, "stderr") != 0)
+    return false;
+
+  // The statusline is the only writer that races Python's terminal output.
+  // When it isn't drawing there is nothing to serialize against, so keep the
+  // normal wrapping and skip the reader thread and pipe.
+  if (!m_debugger.StatuslineSupported())
+    return false;
+
+  // Only the debugger's own terminal races the statusline. A redirect to a
+  // pipe or user file (a different descriptor) is wrapped normally.
+  lldb::FileSP debugger_file =
+      is_stdout ? m_debugger.GetOutputFileSP() : m_debugger.GetErrorFileSP();
+  int fd = file.GetDescriptor();
+  if (!debugger_file || fd == File::kInvalidDescriptor ||
+      fd != debugger_file->GetDescriptor())
+    return false;
+
+  std::unique_ptr<SessionIORedirect> &redirect =
+      is_stdout ? m_stdout_redirect : m_stderr_redirect;
+  redirect = SessionIORedirect::Create(m_debugger.GetID(), is_stdout);
+  if (!redirect)
+    return false;
+
+  // Line-buffer the wrapper so each print() reaches the reader (and the
+  // terminal) promptly: the pipe descriptor is not a tty, so the default
+  // buffering would hold output back until the buffer filled.
+  PyObject *pipe_file = PyFile_FromFd(
+      redirect->GetWriteDescriptor(), nullptr, mode, /*buffering=*/1,
+      /*encoding=*/nullptr, /*errors=*/"ignore", /*newline=*/nullptr,
+      /*closefd=*/0);
+  if (!pipe_file) {
+    // Fall back to the raw descriptor. That reopens the statusline race, so
+    // leave a breadcrumb rather than failing silently.
+    LLDB_LOG(GetLog(LLDBLog::Script),
+             "failed to wrap sys.{0} on a synchronized pipe; falling back to "
+             "the unsynchronized terminal descriptor",
+             py_name);
+    PyErr_Clear();
+    redirect.reset();
+    return false;
+  }
+
+  PythonObject new_file(PyRefType::Owned, pipe_file);
+  PythonDictionary &sys_module_dict = GetSysModuleDictionary();
+  save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
+  sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
+  return true;
+}
+
 bool ScriptInterpreterPythonImpl::SetStdHandle(FileSP file_sp,
                                                const char *py_name,
                                                PythonObject &save_file,
-                                               const char *mode) {
+                                               const char *mode,
+                                               bool serialize_terminal_output) {
   if (!file_sp || !*file_sp) {
     save_file.Reset();
     return false;
   }
   File &file = *file_sp;
 
+  // When stdout/stderr point at the debugger's own terminal, route Python's
+  // output through a pipe drained under the output lock so a script's print()
+  // cannot race the statusline. Any other target keeps the normal wrapping.
+  if (serialize_terminal_output &&
+      RedirectTerminalHandleThroughLock(py_name, save_file, mode, file))
+    return true;
+
   // Flush the file before giving it to python to avoid interleaved output.
   file.Flush();
 
@@ -675,22 +849,31 @@ bool ScriptInterpreterPythonImpl::EnterSession(uint16_t on_entry_flags,
     if (on_entry_flags & Locker::NoSTDIN) {
       m_saved_stdin.Reset();
     } else {
-      if (!SetStdHandle(in_sp, "stdin", m_saved_stdin, "r")) {
+      if (!SetStdHandle(in_sp, "stdin", m_saved_stdin, "r",
+                        /*serialize_terminal_output=*/false)) {
         if (top_in_sp)
-          SetStdHandle(top_in_sp, "stdin", m_saved_stdin, "r");
+          SetStdHandle(top_in_sp, "stdin", m_saved_stdin, "r",
+                       /*serialize_terminal_output=*/false);
       }
     }
 
-    if (!SetStdHandle(out_sp, "stdout", m_saved_stdout, "w")) {
+    // Serialize terminal output for every session except those that opt out
+    // with NoOutputRedirect (see the flag for why).
+    const bool serialize_terminal_output =
+        !(on_entry_flags & Locker::NoOutputRedirect);
+
+    if (!SetStdHandle(out_sp, "stdout", m_saved_stdout, "w",
+                      serialize_terminal_output)) {
       if (top_out_sp)
         SetStdHandle(top_out_sp->GetUnlockedFileSP(), "stdout", m_saved_stdout,
-                     "w");
+                     "w", serialize_terminal_output);
     }
 
-    if (!SetStdHandle(err_sp, "stderr", m_saved_stderr, "w")) {
+    if (!SetStdHandle(err_sp, "stderr", m_saved_stderr, "w",
+                      serialize_terminal_output)) {
       if (top_err_sp)
         SetStdHandle(top_err_sp->GetUnlockedFileSP(), "stderr", m_saved_stderr,
-                     "w");
+                     "w", serialize_terminal_output);
     }
   }
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

@@ -200,7 +200,7 @@ public:
 
   bool GetLongHelpForCommandObject(StructuredData::GenericSP cmd_obj_sp,
                                    std::string &dest) override;
-                                   
+
   StructuredData::ObjectSP
   GetOptionsForCommandObject(StructuredData::GenericSP cmd_obj_sp) override;
 
@@ -209,7 +209,7 @@ public:
 
   bool SetOptionValueForCommandObject(StructuredData::GenericSP cmd_obj_sp,
                                       ExecutionContext *exe_ctx,
-                                      llvm::StringRef long_option, 
+                                      llvm::StringRef long_option,
                                       llvm::StringRef value) override;
 
   void OptionParsingStartedForCommandObject(
@@ -273,8 +273,7 @@ public:
   Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                       const char *command_body_text,
                                       StructuredData::ObjectSP extra_args_sp,
-                                      bool uses_extra_args,
-                                      bool is_callback);
+                                      bool uses_extra_args, bool is_callback);
 
   /// Set a one-liner as the callback for the watchpoint.
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
@@ -307,7 +306,12 @@ public:
       AcquireLock = 0x0001,
       InitSession = 0x0002,
       InitGlobals = 0x0004,
-      NoSTDIN = 0x0008
+      NoSTDIN = 0x0008,
+      // Keep sys.stdout/stderr on the real terminal instead of routing them
+      // through the output-lock pipe (see EnterSession). Set by the interactive
+      // interpreter, whose input() needs both the real stdin and stdout for
+      // readline-based line editing and echo.
+      NoOutputRedirect = 0x0010
     };
 
     enum OnLeave {
@@ -404,12 +408,31 @@ public:
 
   bool GetEmbeddedInterpreterModuleObjects();
 
+  /// Point sys.\p py_name at \p file. When \p serialize_terminal_output is true
+  /// and \p file is the debugger's own terminal, the output is routed through a
+  /// lock-synchronized pipe instead (see RedirectTerminalHandleThroughLock) so
+  /// it cannot race the statusline redraw.
   bool SetStdHandle(lldb::FileSP file, const char *py_name,
-                    python::PythonObject &save_file, const char *mode);
+                    python::PythonObject &save_file, const char *mode,
+                    bool serialize_terminal_output);
+
+  /// Pipe-backed sys.stdout/stderr whose writes are serialized against the
+  /// statusline redraw. Defined and documented in the implementation file.
+  class SessionIORedirect;
+
+  /// If \p file is the debugger's own terminal, point sys.\p py_name at a
+  /// pipe-backed file whose writes are serialized through the output lock (see
+  /// SessionIORedirect) and return true. Return false to let SetStdHandle wrap
+  /// \p file normally.
+  bool RedirectTerminalHandleThroughLock(const char *py_name,
+                                         python::PythonObject &save_file,
+                                         const char *mode, File &file);
 
   python::PythonObject m_saved_stdin;
   python::PythonObject m_saved_stdout;
   python::PythonObject m_saved_stderr;
+  std::unique_ptr<SessionIORedirect> m_stdout_redirect;
+  std::unique_ptr<SessionIORedirect> m_stderr_redirect;
   python::PythonModule m_main_module;
   python::PythonDictionary m_session_dict;
   python::PythonDictionary m_sys_module_dict;
@@ -458,7 +481,8 @@ public:
             m_python,
             ScriptInterpreterPythonImpl::Locker::AcquireLock |
                 ScriptInterpreterPythonImpl::Locker::InitSession |
-                ScriptInterpreterPythonImpl::Locker::InitGlobals,
+                ScriptInterpreterPythonImpl::Locker::InitGlobals |
+                ScriptInterpreterPythonImpl::Locker::NoOutputRedirect,
             ScriptInterpreterPythonImpl::Locker::FreeAcquiredLock |
                 ScriptInterpreterPythonImpl::Locker::TearDownSession);
 

lldb/test/API/functionalities/statusline/TestStatusline.py

@@ -121,6 +121,52 @@ class TestStatusline(PExpectTest):
 
         self.expect("set set show-statusline true", ["no target"])
 
+    def test_scripted_command_output_not_eaten(self):
+        """A scripted command's print() output must be serialized with the
+        statusline redraw. The statusline brackets its redraw with a cursor
+        save (ESC 7) and restore (ESC 8); if command output lands in between,
+        the restore rewinds the cursor back over it and the terminal
+        overwrites it, so the output is non-deterministically eaten. Assert
+        that no command output appears inside a save/restore pair."""
+        self.launch(use_colors=False)
+        self.resize()
+        self.expect("settings set show-statusline true", ["no target"])
+
+        flood = os.path.join(
+            os.path.dirname(os.path.realpath(__file__)), "statusline_flood.py"
+        )
+        self.expect('command script import "{}"'.format(flood))
+
+        tee = CaptureTee()
+        self.child.logfile_read = tee
+        self.child.sendline("statusline_flood")
+        self.child.expect("MARKER_0049")
+        self.child.expect("(lldb)")
+        self.child.logfile_read = None
+
+        data = tee.data
+        # The test is only meaningful if the statusline actually redrew and the
+        # command actually produced output.
+        self.assertIn(b"\x1b7", data)
+        self.assertIn(b"MARKER_0049", data)
+
+        # No command output may appear between a cursor save (ESC 7) and its
+        # matching restore (ESC 8); that would mean the two writers interleaved.
+        pos = 0
+        while True:
+            save = data.find(b"\x1b7", pos)
+            if save == -1:
+                break
+            restore = data.find(b"\x1b8", save)
+            if restore == -1:
+                break
+            self.assertNotIn(
+                b"MARKER_",
+                data[save + 2 : restore],
+                "scripted command output was spliced into a statusline redraw",
+            )
+            pos = restore + 2
+
     @skipIfEditlineSupportMissing
     def test_resize(self):
         """Test that move the cursor when resizing."""

lldb/test/API/functionalities/statusline/statusline_flood.py

@@ -0,0 +1,17 @@
+"""Helper for TestStatusline.test_scripted_command_output_not_eaten.
+
+Registers a command that floods output while emitting progress events, so the
+statusline redraws (on the event thread) concurrently with the command output.
+"""
+
+import lldb
+
+
+@lldb.command("statusline_flood")
+def statusline_flood(debugger, command, result, internal_dict):
+    count = 50
+    progress = lldb.SBProgress("flood", "working", count, debugger)
+    for i in range(count):
+        print("MARKER_{:04d}".format(i), flush=True)
+        progress.Increment(1, "step {}".format(i))
+    progress.Finalize()