This revision was automatically updated to reflect the committed changes.
Closed by commit rGd79273c941d5: [lldb/ScriptInterpreter] Extract IO 
redirection logic (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D82396?vs=273068&id=273429#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82396/new/

https://reviews.llvm.org/D82396

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -42,6 +42,7 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatAdapters.h"
 
@@ -895,15 +896,6 @@
   return m_run_one_line_function.IsValid();
 }
 
-static void ReadThreadBytesReceived(void *baton, const void *src,
-                                    size_t src_len) {
-  if (src && src_len) {
-    Stream *strm = (Stream *)baton;
-    strm->Write(src, src_len);
-    strm->Flush();
-  }
-}
-
 bool ScriptInterpreterPythonImpl::ExecuteOneLine(
     llvm::StringRef command, CommandReturnObject *result,
     const ExecuteScriptOptions &options) {
@@ -919,77 +911,23 @@
     // another string to pass to PyRun_SimpleString messes up the escaping.  So
     // we use the following more complicated method to pass the command string
     // directly down to Python.
-    Debugger &debugger = m_debugger;
-
-    FileSP input_file_sp;
-    StreamFileSP output_file_sp;
-    StreamFileSP error_file_sp;
-    Communication output_comm(
-        "lldb.ScriptInterpreterPythonImpl.ExecuteOneLine.comm");
-    bool join_read_thread = false;
-    if (options.GetEnableIO()) {
-      if (result) {
-        input_file_sp = debugger.GetInputFileSP();
-        // Set output to a temporary file so we can forward the results on to
-        // the result object
-
-        Pipe pipe;
-        Status pipe_result = pipe.CreateNew(false);
-        if (pipe_result.Success()) {
-#if defined(_WIN32)
-          lldb::file_t read_file = pipe.GetReadNativeHandle();
-          pipe.ReleaseReadFileDescriptor();
-          std::unique_ptr<ConnectionGenericFile> conn_up(
-              new ConnectionGenericFile(read_file, true));
-#else
-          std::unique_ptr<ConnectionFileDescriptor> conn_up(
-              new ConnectionFileDescriptor(pipe.ReleaseReadFileDescriptor(),
-                                           true));
-#endif
-          if (conn_up->IsConnected()) {
-            output_comm.SetConnection(std::move(conn_up));
-            output_comm.SetReadThreadBytesReceivedCallback(
-                ReadThreadBytesReceived, &result->GetOutputStream());
-            output_comm.StartReadThread();
-            join_read_thread = true;
-            FILE *outfile_handle =
-                fdopen(pipe.ReleaseWriteFileDescriptor(), "w");
-            output_file_sp = std::make_shared<StreamFile>(outfile_handle, true);
-            error_file_sp = output_file_sp;
-            if (outfile_handle)
-              ::setbuf(outfile_handle, nullptr);
-
-            result->SetImmediateOutputFile(
-                debugger.GetOutputStream().GetFileSP());
-            result->SetImmediateErrorFile(
-                debugger.GetErrorStream().GetFileSP());
-          }
-        }
-      }
-      if (!input_file_sp || !output_file_sp || !error_file_sp)
-        debugger.AdoptTopIOHandlerFilesIfInvalid(input_file_sp, output_file_sp,
-                                                 error_file_sp);
-    } else {
-      auto nullin = FileSystem::Instance().Open(
-                                  FileSpec(FileSystem::DEV_NULL),
-                                  File::eOpenOptionRead);
-      auto nullout = FileSystem::Instance().Open(
-                                  FileSpec(FileSystem::DEV_NULL),
-                                  File::eOpenOptionWrite);
-      if (!nullin) {
-        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
-                                       llvm::fmt_consume(nullin.takeError()));
-        return false;
-      }
-      if (!nullout) {
-        result->AppendErrorWithFormatv("failed to open /dev/null: {0}\n",
-                                       llvm::fmt_consume(nullout.takeError()));
-        return false;
-      }
-      input_file_sp = std::move(nullin.get());
-      error_file_sp = output_file_sp = std::make_shared<StreamFile>(std::move(nullout.get()));
+    llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+        io_redirect_or_error =
+            options.GetEnableIO()
+                ? ScriptInterpreterIORedirect::Create(m_debugger, result)
+                : ScriptInterpreterIORedirect::Create();
+    if (!io_redirect_or_error) {
+      if (result)
+        result->AppendErrorWithFormatv(
+            "failed to redirect I/O: {0}\n",
+            llvm::fmt_consume(io_redirect_or_error.takeError()));
+      else
+        llvm::consumeError(io_redirect_or_error.takeError());
+      return false;
     }
 
+    ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error;
+
     bool success = false;
     {
       // WARNING!  It's imperative that this RAII scope be as tight as
@@ -1005,8 +943,9 @@
           Locker::AcquireLock | Locker::InitSession |
               (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
               ((result && result->GetInteractive()) ? 0 : Locker::NoSTDIN),
-          Locker::FreeAcquiredLock | Locker::TearDownSession, input_file_sp,
-          output_file_sp->GetFileSP(), error_file_sp->GetFileSP());
+          Locker::FreeAcquiredLock | Locker::TearDownSession,
+          io_redirect.GetInputFile(), io_redirect.GetOutputFile(),
+          io_redirect.GetErrorFile());
 
       // Find the correct script interpreter dictionary in the main module.
       PythonDictionary &session_dict = GetSessionDictionary();
@@ -1032,21 +971,7 @@
         }
       }
 
-      // Flush our output and error file handles
-      output_file_sp->Flush();
-      error_file_sp->Flush();
-    }
-
-    if (join_read_thread) {
-      // Close the write end of the pipe since we are done with our one line
-      // script. This should cause the read thread that output_comm is using to
-      // exit
-      output_file_sp->GetFile().Close();
-      // The close above should cause this thread to exit when it gets to the
-      // end of file, so let it get all its data
-      output_comm.JoinReadThread();
-      // Now we can close the read end of the pipe
-      output_comm.Disconnect();
+      io_redirect.Flush();
     }
 
     if (success)
Index: lldb/source/Interpreter/ScriptInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/ScriptInterpreter.cpp
+++ lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -8,10 +8,14 @@
 
 #include "lldb/Interpreter/ScriptInterpreter.h"
 
+#include <memory>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string>
 
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/Pipe.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/Status.h"
@@ -104,3 +108,113 @@
 ScriptInterpreter::AcquireInterpreterLock() {
   return std::make_unique<ScriptInterpreterLocker>();
 }
+
+static void ReadThreadBytesReceived(void *baton, const void *src,
+                                    size_t src_len) {
+  if (src && src_len) {
+    Stream *strm = (Stream *)baton;
+    strm->Write(src, src_len);
+    strm->Flush();
+  }
+}
+
+llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ScriptInterpreterIORedirect::Create() {
+  auto nullin = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
+                                            File::eOpenOptionRead);
+  if (!nullin)
+    return nullin.takeError();
+
+  auto nullout = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
+                                             File::eOpenOptionWrite);
+  if (!nullout)
+    return nullin.takeError();
+
+  return std::unique_ptr<ScriptInterpreterIORedirect>(
+      new ScriptInterpreterIORedirect(std::move(*nullin), std::move(*nullout)));
+}
+
+llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ScriptInterpreterIORedirect::Create(Debugger &debugger,
+                                    CommandReturnObject *result) {
+  return std::unique_ptr<ScriptInterpreterIORedirect>(
+      new ScriptInterpreterIORedirect(debugger, result));
+}
+
+ScriptInterpreterIORedirect::ScriptInterpreterIORedirect(
+    std::unique_ptr<File> input, std::unique_ptr<File> output)
+    : m_input_file_sp(std::move(input)),
+      m_output_file_sp(std::make_shared<StreamFile>(std::move(output))),
+      m_error_file_sp(m_output_file_sp),
+      m_communication("lldb.ScriptInterpreterIORedirect.comm"),
+      m_disconnect(false) {}
+
+ScriptInterpreterIORedirect::ScriptInterpreterIORedirect(
+    Debugger &debugger, CommandReturnObject *result)
+    : m_communication("lldb.ScriptInterpreterIORedirect.comm"),
+      m_disconnect(false) {
+
+  if (result) {
+    m_input_file_sp = debugger.GetInputFileSP();
+
+    Pipe pipe;
+    Status pipe_result = pipe.CreateNew(false);
+#if defined(_WIN32)
+    lldb::file_t read_file = pipe.GetReadNativeHandle();
+    pipe.ReleaseReadFileDescriptor();
+    std::unique_ptr<ConnectionGenericFile> conn_up =
+        std::make_unique<ConnectionGenericFile>(read_file, true);
+#else
+    std::unique_ptr<ConnectionFileDescriptor> conn_up =
+        std::make_unique<ConnectionFileDescriptor>(
+            pipe.ReleaseReadFileDescriptor(), true);
+#endif
+
+    if (conn_up->IsConnected()) {
+      m_communication.SetConnection(std::move(conn_up));
+      m_communication.SetReadThreadBytesReceivedCallback(
+          ReadThreadBytesReceived, &result->GetOutputStream());
+      m_communication.StartReadThread();
+      m_disconnect = true;
+
+      FILE *outfile_handle = fdopen(pipe.ReleaseWriteFileDescriptor(), "w");
+      m_output_file_sp = std::make_shared<StreamFile>(outfile_handle, true);
+      m_error_file_sp = m_output_file_sp;
+      if (outfile_handle)
+        ::setbuf(outfile_handle, nullptr);
+
+      result->SetImmediateOutputFile(debugger.GetOutputStream().GetFileSP());
+      result->SetImmediateErrorFile(debugger.GetErrorStream().GetFileSP());
+    }
+  }
+
+  if (!m_input_file_sp || !m_output_file_sp || !m_error_file_sp)
+    debugger.AdoptTopIOHandlerFilesIfInvalid(m_input_file_sp, m_output_file_sp,
+                                             m_error_file_sp);
+}
+
+void ScriptInterpreterIORedirect::Flush() {
+  if (m_output_file_sp)
+    m_output_file_sp->Flush();
+  if (m_error_file_sp)
+    m_error_file_sp->Flush();
+}
+
+ScriptInterpreterIORedirect::~ScriptInterpreterIORedirect() {
+  if (!m_disconnect)
+    return;
+
+  assert(m_output_file_sp);
+  assert(m_error_file_sp);
+  assert(m_output_file_sp == m_error_file_sp);
+
+  // Close the write end of the pipe since we are done with our one line
+  // script. This should cause the read thread that output_comm is using to
+  // exit.
+  m_output_file_sp->GetFile().Close();
+  // The close above should cause this thread to exit when it gets to the end
+  // of file, so let it get all its data.
+  m_communication.JoinReadThread();
+  // Now we can close the read end of the pipe.
+  m_communication.Disconnect();
+}
Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -9,16 +9,16 @@
 #ifndef LLDB_INTERPRETER_SCRIPTINTERPRETER_H
 #define LLDB_INTERPRETER_SCRIPTINTERPRETER_H
 
-#include "lldb/lldb-private.h"
-
 #include "lldb/Breakpoint/BreakpointOptions.h"
+#include "lldb/Core/Communication.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/SearchFilter.h"
+#include "lldb/Core/StreamFile.h"
+#include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
-
-#include "lldb/Host/PseudoTerminal.h"
+#include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
@@ -34,6 +34,37 @@
   operator=(const ScriptInterpreterLocker &) = delete;
 };
 
+class ScriptInterpreterIORedirect {
+public:
+  /// Create an IO redirect with /dev/null as input, output and error file.
+  static llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>> Create();
+
+  /// Create an IO redirect that redirects the output to the command return
+  /// object if set or to the debugger otherwise.
+  static llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+  Create(Debugger &debugger, CommandReturnObject *result);
+
+  ~ScriptInterpreterIORedirect();
+
+  lldb::FileSP GetInputFile() const { return m_input_file_sp; }
+  lldb::FileSP GetOutputFile() const { return m_output_file_sp->GetFileSP(); }
+  lldb::FileSP GetErrorFile() const { return m_error_file_sp->GetFileSP(); }
+
+  /// Flush our output and error file handles.
+  void Flush();
+
+private:
+  ScriptInterpreterIORedirect(std::unique_ptr<File> input,
+                              std::unique_ptr<File> output);
+  ScriptInterpreterIORedirect(Debugger &debugger, CommandReturnObject *result);
+
+  lldb::FileSP m_input_file_sp;
+  lldb::StreamFileSP m_output_file_sp;
+  lldb::StreamFileSP m_error_file_sp;
+  Communication m_communication;
+  bool m_disconnect;
+};
+
 class ScriptInterpreter : public PluginInterface {
 public:
   enum ScriptReturnType {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to