[lldb] Synchronize the debuggers output & error streams

This patch improves the synchronization of the debugger's output and error
streams using two new abstractions: `LockableStreamFile` and
`LockedStreamFile`.

 - `LockableStreamFile` is a wrapper around a `StreamFile` and a mutex. Client
   cannot use the `StreamFile` without calling `Lock`, which returns a
   `LockedStreamFile`.

 - `LockedStreamFile` is an RAII object that locks the stream for the duration
   of its existence.  As long as you hold on to the returned object you are
   permitted to write to the stream. The destruction of the object
   automatically flush the output stream.

Author: JDevlieghere

lldb/include/lldb/Core/Debugger.h

@@ -131,13 +131,16 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   void SetAsyncExecution(bool async);
 
-  File &GetInputFile() { return *m_input_file_sp; }
-
   lldb::FileSP GetInputFileSP() { return m_input_file_sp; }
+  File &GetInputFile() { return *m_input_file_sp; }
 
-  lldb::FileSP GetOutputFileSP() { return m_output_stream_sp->GetFileSP(); }
+  lldb::FileSP GetOutputFileSP() {
+    return m_output_stream_sp->GetUnlockedFileSP();
+  }
 
-  lldb::FileSP GetErrorFileSP() { return m_error_stream_sp->GetFileSP(); }
+  lldb::FileSP GetErrorFileSP() {
+    return m_error_stream_sp->GetUnlockedFileSP();
+  }
 
   repro::DataRecorder *GetInputRecorder();
 
@@ -198,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,
@@ -649,8 +652,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
   /// should not be used directly. Use GetAsyncOutputStream and
   /// GetAsyncErrorStream instead.
   /// @{
-  lldb::StreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
-  lldb::StreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
+  lldb::LockableStreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
+  lldb::LockableStreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
   /// @}
 
   void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
@@ -693,8 +696,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

@@ -53,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();
 
@@ -112,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; }
 
@@ -155,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;
@@ -330,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,
@@ -345,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/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, bool color, 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();
 
@@ -237,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);
@@ -392,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;
@@ -411,7 +417,6 @@ class Editline {
   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

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -116,8 +116,12 @@ class ScriptInterpreterIORedirect {
   ~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(); }
+  lldb::FileSP GetOutputFile() const {
+    return m_output_file_sp->GetUnlockedFileSP();
+  }
+  lldb::FileSP GetErrorFile() const {
+    return m_error_file_sp->GetUnlockedFileSP();
+  }
 
   /// Flush our output and error file handles.
   void Flush();
@@ -128,8 +132,9 @@ class ScriptInterpreterIORedirect {
   ScriptInterpreterIORedirect(Debugger &debugger, CommandReturnObject *result);
 
   lldb::FileSP m_input_file_sp;
-  lldb::StreamFileSP m_output_file_sp;
-  lldb::StreamFileSP m_error_file_sp;
+  lldb::LockableStreamFileSP m_output_file_sp;
+  lldb::LockableStreamFileSP m_error_file_sp;
+  LockableStreamFile::Mutex m_output_mutex;
   ThreadedCommunication m_communication;
   bool m_disconnect;
 };
@@ -478,7 +483,7 @@ class ScriptInterpreter : public PluginInterface {
     dest.clear();
     return false;
   }
-  
+
   virtual StructuredData::ObjectSP
   GetOptionsForCommandObject(StructuredData::GenericSP cmd_obj_sp) {
     return {};
@@ -488,9 +493,9 @@ class ScriptInterpreter : public PluginInterface {
   GetArgumentsForCommandObject(StructuredData::GenericSP cmd_obj_sp) {
     return {};
   }
-  
+
   virtual bool SetOptionValueForCommandObject(
-      StructuredData::GenericSP cmd_obj_sp, ExecutionContext *exe_ctx, 
+      StructuredData::GenericSP cmd_obj_sp, ExecutionContext *exe_ctx,
       llvm::StringRef long_option, llvm::StringRef value) {
     return false;
   }

lldb/include/lldb/lldb-forward.h

@@ -215,6 +215,7 @@ class StoppointCallbackContext;
 class Stream;
 class StreamFile;
 class StreamString;
+class LockableStreamFile;
 class StringList;
 class StringTableReader;
 class StructuredDataImpl;
@@ -432,6 +433,7 @@ typedef std::unique_ptr<lldb_private::StackFrameRecognizerManager>
 typedef std::shared_ptr<lldb_private::StopInfo> StopInfoSP;
 typedef std::shared_ptr<lldb_private::Stream> StreamSP;
 typedef std::shared_ptr<lldb_private::StreamFile> StreamFileSP;
+typedef std::shared_ptr<lldb_private::LockableStreamFile> LockableStreamFileSP;
 typedef std::shared_ptr<lldb_private::StringSummaryFormat>
     StringTypeSummaryImplSP;
 typedef std::unique_ptr<lldb_private::StructuredDataImpl> StructuredDataImplUP;

lldb/source/Commands/CommandObjectBreakpointCommand.cpp

@@ -193,10 +193,12 @@ are no syntax errors may indicate that a function was declared but never called.
   Options *GetOptions() override { return &m_all_options; }
 
   void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
-    StreamFileSP output_sp(io_handler.GetOutputStreamFileSP());
-    if (output_sp && interactive) {
-      output_sp->PutCString(g_reader_instructions);
-      output_sp->Flush();
+    if (interactive) {
+      if (lldb::LockableStreamFileSP output_sp =
+              io_handler.GetOutputStreamFileSP()) {
+        LockedStreamFile locked_stream = output_sp->Lock();
+        locked_stream.PutCString(g_reader_instructions);
+      }
     }
   }