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

Cherrypick commits to synchronize the debugger output and error stream

Author: JDevlieghere

lldb/include/lldb/Core/Debugger.h

@@ -132,20 +132,15 @@ 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; }
 
-  File &GetOutputFile() { return m_output_stream_sp->GetFile(); }
-
-  File &GetErrorFile() { return m_error_stream_sp->GetFile(); }
-
-  StreamFile &GetOutputStream() { return *m_output_stream_sp; }
+  lldb::FileSP GetOutputFileSP() {
+    return m_output_stream_sp->GetUnlockedFileSP();
+  }
 
-  StreamFile &GetErrorStream() { return *m_error_stream_sp; }
+  lldb::FileSP GetErrorFileSP() {
+    return m_error_stream_sp->GetUnlockedFileSP();
+  }
 
   repro::DataRecorder *GetInputRecorder();
 
@@ -161,9 +156,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   void RestoreInputTerminalState();
 
-  lldb::StreamSP GetAsyncOutputStream();
+  lldb::StreamUP GetAsyncOutputStream();
 
-  lldb::StreamSP GetAsyncErrorStream();
+  lldb::StreamUP GetAsyncErrorStream();
 
   CommandInterpreter &GetCommandInterpreter() {
     assert(m_command_interpreter_up.get());
@@ -206,8 +201,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::StreamFileSP &out,
-                                       lldb::StreamFileSP &err);
+                                       lldb::LockableStreamFileSP &out,
+                                       lldb::LockableStreamFileSP &err);
 
   /// Run the given IO handler and return immediately.
   void RunIOHandlerAsync(const lldb::IOHandlerSP &reader_sp,
@@ -662,6 +657,14 @@ 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);
 
@@ -702,8 +705,9 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   // these should never be NULL
   lldb::FileSP m_input_file_sp;
-  lldb::StreamFileSP m_output_stream_sp;
-  lldb::StreamFileSP m_error_stream_sp;
+  lldb::LockableStreamFileSP m_output_stream_sp;
+  lldb::LockableStreamFileSP m_error_stream_sp;
+  LockableStreamFile::Mutex m_output_mutex;
 
   /// Used for shadowing the input file when capturing a reproducer.
   repro::DataRecorder *m_input_recorder;

lldb/include/lldb/Core/IOHandler.h

@@ -32,11 +32,6 @@ 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 {
@@ -58,8 +53,9 @@ class IOHandler {
   IOHandler(Debugger &debugger, IOHandler::Type type);
 
   IOHandler(Debugger &debugger, IOHandler::Type type,
-            const lldb::FileSP &input_sp, const lldb::StreamFileSP &output_sp,
-            const lldb::StreamFileSP &error_sp, uint32_t flags);
+            const lldb::FileSP &input_sp,
+            const lldb::LockableStreamFileSP &output_sp,
+            const lldb::LockableStreamFileSP &error_sp, uint32_t flags);
 
   virtual ~IOHandler();
 
@@ -117,17 +113,11 @@ class IOHandler {
 
   int GetErrorFD();
 
-  FILE *GetInputFILE();
-
-  FILE *GetOutputFILE();
-
-  FILE *GetErrorFILE();
-
   lldb::FileSP GetInputFileSP();
 
-  lldb::StreamFileSP GetOutputStreamFileSP();
+  lldb::LockableStreamFileSP GetOutputStreamFileSP();
 
-  lldb::StreamFileSP GetErrorStreamFileSP();
+  lldb::LockableStreamFileSP GetErrorStreamFileSP();
 
   Debugger &GetDebugger() { return m_debugger; }
 
@@ -160,14 +150,11 @@ 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::StreamFileSP m_output_sp;
-  lldb::StreamFileSP m_error_sp;
-  std::recursive_mutex m_output_mutex;
+  lldb::LockableStreamFileSP m_output_sp;
+  lldb::LockableStreamFileSP m_error_sp;
   Predicate<bool> m_popped;
   Flags m_flags;
   Type m_type;
@@ -335,8 +322,8 @@ class IOHandlerEditline : public IOHandler {
 
   IOHandlerEditline(Debugger &debugger, IOHandler::Type type,
                     const lldb::FileSP &input_sp,
-                    const lldb::StreamFileSP &output_sp,
-                    const lldb::StreamFileSP &error_sp, uint32_t flags,
+                    const lldb::LockableStreamFileSP &output_sp,
+                    const lldb::LockableStreamFileSP &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,
@@ -350,9 +337,10 @@ class IOHandlerEditline : public IOHandler {
                     IOHandlerDelegate &) = delete;
 
   IOHandlerEditline(Debugger &, IOHandler::Type, const lldb::FileSP &,
-                    const lldb::StreamFileSP &, const lldb::StreamFileSP &,
-                    uint32_t, const char *, const char *, const char *, bool,
-                    bool, uint32_t, IOHandlerDelegate &) = delete;
+                    const lldb::LockableStreamFileSP &,
+                    const lldb::LockableStreamFileSP &, uint32_t, const char *,
+                    const char *, const char *, bool, bool, uint32_t,
+                    IOHandlerDelegate &) = delete;
 
   ~IOHandlerEditline() override;
 

lldb/include/lldb/Core/IOHandlerCursesGUI.h

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

lldb/include/lldb/Core/StreamAsynchronousIO.h

@@ -18,9 +18,17 @@
 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:
-  StreamAsynchronousIO(Debugger &debugger, bool for_stdout, bool colors);
+  enum ForSTDOUT : bool {
+    STDOUT = true,
+    STDERR = false,
+  };
+
+  StreamAsynchronousIO(Debugger &debugger, ForSTDOUT for_stdout);
 
   ~StreamAsynchronousIO() override;
 
@@ -32,7 +40,7 @@ class StreamAsynchronousIO : public Stream {
 private:
   Debugger &m_debugger;
   std::string m_data;
-  bool m_for_stdout;
+  ForSTDOUT m_for_stdout;
 };
 
 } // namespace lldb_private

lldb/include/lldb/Host/Editline.h

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