Merge pull request swiftlang#10369 from swiftlang/revert-10368-cherrypick-output-error-stream-locking

Revert "Cherrypick commits to synchronize the debugger output and error stream"

Author: milseman

lldb/include/lldb/Core/Debugger.h

@@ -132,15 +132,20 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   void SetAsyncExecution(bool async);
 
   lldb::FileSP GetInputFileSP() { return m_input_file_sp; }
+
+  lldb::StreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
+
+  lldb::StreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
+
   File &GetInputFile() { return *m_input_file_sp; }
 
-  lldb::FileSP GetOutputFileSP() {
-    return m_output_stream_sp->GetUnlockedFileSP();
-  }
+  File &GetOutputFile() { return m_output_stream_sp->GetFile(); }
 
-  lldb::FileSP GetErrorFileSP() {
-    return m_error_stream_sp->GetUnlockedFileSP();
-  }
+  File &GetErrorFile() { return m_error_stream_sp->GetFile(); }
+
+  StreamFile &GetOutputStream() { return *m_output_stream_sp; }
+
+  StreamFile &GetErrorStream() { return *m_error_stream_sp; }
 
   repro::DataRecorder *GetInputRecorder();
 
@@ -156,9 +161,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   void RestoreInputTerminalState();
 
-  lldb::StreamUP GetAsyncOutputStream();
+  lldb::StreamSP GetAsyncOutputStream();
 
-  lldb::StreamUP GetAsyncErrorStream();
+  lldb::StreamSP GetAsyncErrorStream();
 
   CommandInterpreter &GetCommandInterpreter() {
     assert(m_command_interpreter_up.get());
@@ -201,8 +206,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   // If any of the streams are not set, set them to the in/out/err stream of
   // the top most input reader to ensure they at least have something
   void AdoptTopIOHandlerFilesIfInvalid(lldb::FileSP &in,
-                                       lldb::LockableStreamFileSP &out,
-                                       lldb::LockableStreamFileSP &err);
+                                       lldb::StreamFileSP &out,
+                                       lldb::StreamFileSP &err);
 
   /// Run the given IO handler and return immediately.
   void RunIOHandlerAsync(const lldb::IOHandlerSP &reader_sp,
@@ -657,14 +662,6 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   void PrintProgress(const ProgressEventData &data);
 
-  /// Except for Debugger and IOHandler, GetOutputStreamSP and GetErrorStreamSP
-  /// should not be used directly. Use GetAsyncOutputStream and
-  /// GetAsyncErrorStream instead.
-  /// @{
-  lldb::LockableStreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
-  lldb::LockableStreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
-  /// @}
-
   void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
                      bool cancel_top_handler = true);
 
@@ -705,9 +702,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   // these should never be NULL
   lldb::FileSP m_input_file_sp;
-  lldb::LockableStreamFileSP m_output_stream_sp;
-  lldb::LockableStreamFileSP m_error_stream_sp;
-  LockableStreamFile::Mutex m_output_mutex;
+  lldb::StreamFileSP m_output_stream_sp;
+  lldb::StreamFileSP m_error_stream_sp;
 
   /// Used for shadowing the input file when capturing a reproducer.
   repro::DataRecorder *m_input_recorder;

lldb/include/lldb/Core/IOHandler.h

@@ -32,6 +32,11 @@ namespace lldb_private {
 class Debugger;
 } // namespace lldb_private
 
+namespace curses {
+class Application;
+typedef std::unique_ptr<Application> ApplicationAP;
+} // namespace curses
+
 namespace lldb_private {
 
 class IOHandler {
@@ -53,9 +58,8 @@ class IOHandler {
   IOHandler(Debugger &debugger, IOHandler::Type type);
 
   IOHandler(Debugger &debugger, IOHandler::Type type,
-            const lldb::FileSP &input_sp,
-            const lldb::LockableStreamFileSP &output_sp,
-            const lldb::LockableStreamFileSP &error_sp, uint32_t flags);
+            const lldb::FileSP &input_sp, const lldb::StreamFileSP &output_sp,
+            const lldb::StreamFileSP &error_sp, uint32_t flags);
 
   virtual ~IOHandler();
 
@@ -113,11 +117,17 @@ class IOHandler {
 
   int GetErrorFD();
 
+  FILE *GetInputFILE();
+
+  FILE *GetOutputFILE();
+
+  FILE *GetErrorFILE();
+
   lldb::FileSP GetInputFileSP();
 
-  lldb::LockableStreamFileSP GetOutputStreamFileSP();
+  lldb::StreamFileSP GetOutputStreamFileSP();
 
-  lldb::LockableStreamFileSP GetErrorStreamFileSP();
+  lldb::StreamFileSP GetErrorStreamFileSP();
 
   Debugger &GetDebugger() { return m_debugger; }
 
@@ -150,11 +160,14 @@ class IOHandler {
 
   virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
 
+  std::recursive_mutex &GetOutputMutex() { return m_output_mutex; }
+
 protected:
   Debugger &m_debugger;
   lldb::FileSP m_input_sp;
-  lldb::LockableStreamFileSP m_output_sp;
-  lldb::LockableStreamFileSP m_error_sp;
+  lldb::StreamFileSP m_output_sp;
+  lldb::StreamFileSP m_error_sp;
+  std::recursive_mutex m_output_mutex;
   Predicate<bool> m_popped;
   Flags m_flags;
   Type m_type;
@@ -322,8 +335,8 @@ class IOHandlerEditline : public IOHandler {
 
   IOHandlerEditline(Debugger &debugger, IOHandler::Type type,
                     const lldb::FileSP &input_sp,
-                    const lldb::LockableStreamFileSP &output_sp,
-                    const lldb::LockableStreamFileSP &error_sp, uint32_t flags,
+                    const lldb::StreamFileSP &output_sp,
+                    const lldb::StreamFileSP &error_sp, uint32_t flags,
                     const char *editline_name, // Used for saving history files
                     llvm::StringRef prompt, llvm::StringRef continuation_prompt,
                     bool multi_line, bool color,
@@ -337,10 +350,9 @@ class IOHandlerEditline : public IOHandler {
                     IOHandlerDelegate &) = delete;
 
   IOHandlerEditline(Debugger &, IOHandler::Type, const lldb::FileSP &,
-                    const lldb::LockableStreamFileSP &,
-                    const lldb::LockableStreamFileSP &, uint32_t, const char *,
-                    const char *, const char *, bool, bool, uint32_t,
-                    IOHandlerDelegate &) = delete;
+                    const lldb::StreamFileSP &, const lldb::StreamFileSP &,
+                    uint32_t, const char *, const char *, const char *, bool,
+                    bool, uint32_t, IOHandlerDelegate &) = delete;
 
   ~IOHandlerEditline() override;
 

lldb/include/lldb/Core/IOHandlerCursesGUI.h

@@ -12,9 +12,6 @@
 #include "lldb/Core/IOHandler.h"
 
 namespace lldb_private {
-namespace curses {
-class Application;
-} // namespace curses
 
 class IOHandlerCursesGUI : public IOHandler {
 public:
@@ -37,7 +34,7 @@ class IOHandlerCursesGUI : public IOHandler {
   void TerminalSizeChanged() override;
 
 protected:
-  std::unique_ptr<curses::Application> m_app_up;
+  curses::ApplicationAP m_app_ap;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Core/StreamAsynchronousIO.h

@@ -18,17 +18,9 @@
 namespace lldb_private {
 class Debugger;
 
-/// A stream meant for asynchronously printing output. Output is buffered until
-/// the stream is flushed or destroyed. Printing is handled by the currently
-/// active IOHandler, or the debugger's output or error stream if there is none.
 class StreamAsynchronousIO : public Stream {
 public:
-  enum ForSTDOUT : bool {
-    STDOUT = true,
-    STDERR = false,
-  };
-
-  StreamAsynchronousIO(Debugger &debugger, ForSTDOUT for_stdout);
+  StreamAsynchronousIO(Debugger &debugger, bool for_stdout, bool colors);
 
   ~StreamAsynchronousIO() override;
 
@@ -40,7 +32,7 @@ class StreamAsynchronousIO : public Stream {
 private:
   Debugger &m_debugger;
   std::string m_data;
-  ForSTDOUT m_for_stdout;
+  bool m_for_stdout;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Host/Editline.h

@@ -34,7 +34,6 @@
 #include <sstream>
 #include <vector>
 
-#include "lldb/Host/StreamFile.h"
 #include "lldb/lldb-private.h"
 
 #if !defined(_WIN32) && !defined(__ANDROID__)
@@ -152,9 +151,8 @@ using namespace line_editor;
 /// facility.  Both single- and multi-line editing are supported.
 class Editline {
 public:
-  Editline(const char *editor_name, FILE *input_file,
-           lldb::LockableStreamFileSP output_stream_sp,
-           lldb::LockableStreamFileSP error_stream_sp, bool color);
+  Editline(const char *editor_name, FILE *input_file, FILE *output_file,
+           FILE *error_file, std::recursive_mutex &output_mutex);
 
   ~Editline();
 
@@ -214,23 +212,19 @@ class Editline {
   }
 
   void SetPromptAnsiPrefix(std::string prefix) {
-    if (m_color)
-      m_prompt_ansi_prefix = std::move(prefix);
+    m_prompt_ansi_prefix = std::move(prefix);
   }
 
   void SetPromptAnsiSuffix(std::string suffix) {
-    if (m_color)
-      m_prompt_ansi_suffix = std::move(suffix);
+    m_prompt_ansi_suffix = std::move(suffix);
   }
 
   void SetSuggestionAnsiPrefix(std::string prefix) {
-    if (m_color)
-      m_suggestion_ansi_prefix = std::move(prefix);
+    m_suggestion_ansi_prefix = std::move(prefix);
   }
 
   void SetSuggestionAnsiSuffix(std::string suffix) {
-    if (m_color)
-      m_suggestion_ansi_suffix = std::move(suffix);
+    m_suggestion_ansi_suffix = std::move(suffix);
   }
 
   /// Prompts for and reads a single line of user input.
@@ -239,8 +233,7 @@ class Editline {
   /// Prompts for and reads a multi-line batch of user input.
   bool GetLines(int first_line_number, StringList &lines, bool &interrupted);
 
-  void PrintAsync(lldb::LockableStreamFileSP stream_sp, const char *s,
-                  size_t len);
+  void PrintAsync(Stream *stream, const char *s, size_t len);
 
   /// Convert the current input lines into a UTF8 StringList
   StringList GetInputAsStringList(int line_count = UINT32_MAX);
@@ -395,11 +388,8 @@ class Editline {
   volatile std::sig_atomic_t m_terminal_size_has_changed = 0;
   std::string m_editor_name;
   FILE *m_input_file;
-  lldb::LockableStreamFileSP m_output_stream_sp;
-  lldb::LockableStreamFileSP m_error_stream_sp;
-
-  std::optional<LockedStreamFile> m_locked_output;
-
+  FILE *m_output_file;
+  FILE *m_error_file;
   ConnectionFileDescriptor m_input_connection;
 
   IsInputCompleteCallbackType m_is_input_complete_callback;
@@ -410,13 +400,13 @@ class Editline {
   CompleteCallbackType m_completion_callback;
   SuggestionCallbackType m_suggestion_callback;
 
-  bool m_color;
   std::string m_prompt_ansi_prefix;
   std::string m_prompt_ansi_suffix;
   std::string m_suggestion_ansi_prefix;
   std::string m_suggestion_ansi_suffix;
 
   std::size_t m_previous_autosuggestion_size = 0;
+  std::recursive_mutex &m_output_mutex;
 };
 }
 

lldb/include/lldb/Host/File.h

@@ -377,11 +377,6 @@ class File : public IOObject {
 
 class NativeFile : public File {
 public:
-  enum TransferOwnership : bool {
-    Owned = true,
-    Unowned = false,
-  };
-
   NativeFile() : m_descriptor(kInvalidDescriptor), m_stream(kInvalidStream) {}
 
   NativeFile(FILE *fh, bool transfer_ownership)

lldb/include/lldb/Host/StreamFile.h

@@ -13,12 +13,9 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
-#include "lldb/lldb-forward.h"
 
 #include <cstdint>
 #include <cstdio>
-#include <memory>
-#include <mutex>
 
 namespace lldb_private {
 
@@ -55,55 +52,6 @@ class StreamFile : public Stream {
   const StreamFile &operator=(const StreamFile &) = delete;
 };
 
-class LockableStreamFile;
-class LockedStreamFile : public StreamFile {
-public:
-  ~LockedStreamFile() { Flush(); }
-
-  LockedStreamFile(LockedStreamFile &&other)
-      : StreamFile(other.m_file_sp), m_lock(std::move(other.m_lock)) {}
-
-private:
-  LockedStreamFile(std::shared_ptr<File> file, std::recursive_mutex &mutex)
-      : StreamFile(file), m_lock(mutex) {}
-
-  friend class LockableStreamFile;
-
-  std::unique_lock<std::recursive_mutex> m_lock;
-};
-
-class LockableStreamFile {
-public:
-  using Mutex = std::recursive_mutex;
-
-  LockableStreamFile(std::shared_ptr<StreamFile> stream_file_sp, Mutex &mutex)
-      : m_file_sp(stream_file_sp->GetFileSP()), m_mutex(mutex) {}
-  LockableStreamFile(StreamFile &stream_file, Mutex &mutex)
-      : m_file_sp(stream_file.GetFileSP()), m_mutex(mutex) {}
-  LockableStreamFile(FILE *fh, bool transfer_ownership, Mutex &mutex)
-      : m_file_sp(std::make_shared<NativeFile>(fh, transfer_ownership)),
-        m_mutex(mutex) {}
-  LockableStreamFile(std::shared_ptr<File> file_sp, Mutex &mutex)
-      : m_file_sp(file_sp), m_mutex(mutex) {}
-
-  LockedStreamFile Lock() { return LockedStreamFile(m_file_sp, m_mutex); }
-
-  /// Unsafe accessors to get the underlying File without a lock. Exists for
-  /// legacy reasons.
-  /// @{
-  File &GetUnlockedFile() { return *m_file_sp; }
-  std::shared_ptr<File> GetUnlockedFileSP() { return m_file_sp; }
-  /// @}
-
-protected:
-  std::shared_ptr<File> m_file_sp;
-  Mutex &m_mutex;
-
-private:
-  LockableStreamFile(const LockableStreamFile &) = delete;
-  const LockableStreamFile &operator=(const LockableStreamFile &) = delete;
-};
-
 } // namespace lldb_private
 
 #endif // LLDB_HOST_STREAMFILE_H