[lldb] Synchronize the output through the IOHandler

[JDevlieghere]

Summary

Alternative to D121500 that does the locking at the IOHandler level.

[labath]

That is roughly what I had in mind, *but* if you look inside Editline class, you'll see that it already declares a std::mutex m_output_mutex variable. It was introduced for the same reason as this mutex here, although it only controls stdout access during command line editing (and not when the command output is printed).

Ideally we wouldn't have two mutexes controlling stdout access. I think it should be possible to delete the mutex in the Editline class, and replace it with (a reference to?) the mutex you define here, but I haven't tried proving to myself that this will work.

[JDevlieghere]

Use the IOHandler's mutex in editline

[JDevlieghere]

Update unit test

[labath]

lldb/include/lldb/Interpreter/CommandInterpreter.h

659

[JDevlieghere]

Landed as 242c574dc03e

lldb/include/lldb/Core/IOHandler.h

public:
   bool GetIsRealTerminal();
 
   void SetPopped(bool b);
 
   void WaitForPop();
 
   virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
 
+  std::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::mutex m_output_mutex;
   repro::DataRecorder *m_data_recorder;
   Predicate<bool> m_popped;
   Flags m_flags;
   Type m_type;
   void *m_user_data;
   bool m_done;
   bool m_active;
 

lldb/include/lldb/Host/Editline.h

 using namespace line_editor;
 
 /// Instances of Editline provide an abstraction over libedit's EditLine
 /// 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_prompts);
+           FILE *error_file, std::mutex &output_mutex, bool color_prompts);
 
   ~Editline();
 
   /// Uses the user data storage of EditLine to retrieve an associated instance
   /// of Editline.
   static Editline *InstanceFor(::EditLine *editline);
 
   /// Sets a string to be used as a prompt, or combined with a line number to
#endif
 
   CompleteCallbackType m_completion_callback;
   SuggestionCallbackType m_suggestion_callback;
 
   std::string m_suggestion_ansi_prefix;
   std::string m_suggestion_ansi_suffix;
 
   std::size_t m_previous_autosuggestion_size = 0;
-  std::mutex m_output_mutex;
+  std::mutex &m_output_mutex;
 };
 }
 
 #endif // #if defined(__cplusplus)
 #endif // LLDB_HOST_EDITLINE_H

lldb/include/lldb/Interpreter/CommandInterpreter.h

private:
   CommandObject *ResolveCommandImpl(std::string &command_line,
                                     CommandReturnObject &result);
 
   void FindCommandsForApropos(llvm::StringRef word, StringList &commands_found,
                               StringList &commands_help,
                               const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
-  void PrintCommandOutput(Stream &stream, llvm::StringRef str);
+  void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
+                          bool is_stdout0);
 
   bool EchoCommandNonInteractive(llvm::StringRef line,
                                  const Flags &io_handler_flags) const;
 
   // A very simple state machine which models the command handling transitions
   enum class CommandHandlingState {
     eIdle,
     eInProgress,

lldb/source/Core/IOHandler.cpp

bool IOHandler::GetIsRealTerminal() {
   return GetInputFileSP() ? GetInputFileSP()->GetIsRealTerminal() : false;
 }
 
 void IOHandler::SetPopped(bool b) { m_popped.SetValue(b, eBroadcastOnChange); }
 
 void IOHandler::WaitForPop() { m_popped.WaitForValueEqualTo(true); }
 
 void IOHandler::PrintAsync(const char *s, size_t len, bool is_stdout) {
+  std::lock_guard<std::mutex> guard(m_output_mutex);
   lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
   stream->Write(s, len);
   stream->Flush();
 }
 
 bool IOHandlerStack::PrintAsync(const char *s, size_t len, bool is_stdout) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (!m_top)

 
 #if LLDB_ENABLE_LIBEDIT
   bool use_editline = false;
 
   use_editline = GetInputFILE() && GetOutputFILE() && GetErrorFILE() &&
                  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
-    m_editline_up = std::make_unique<Editline>(editline_name, GetInputFILE(),
-                                               GetOutputFILE(), GetErrorFILE(),
-                                               m_color_prompts);
+    m_editline_up = std::make_unique<Editline>(
+        editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(),
+        GetOutputMutex(), m_color_prompts);
     m_editline_up->SetIsInputCompleteCallback(
         [this](Editline *editline, StringList &lines) {
           return this->IsInputCompleteCallback(editline, lines);
         });
 
     m_editline_up->SetAutoCompleteCallback([this](CompletionRequest &request) {
       this->AutoCompleteCallback(request);
     });
#if LLDB_ENABLE_LIBEDIT
   if (m_editline_up)
     m_editline_up->Interrupt();
 #endif
 }
 
 void IOHandlerEditline::PrintAsync(const char *s, size_t len, bool is_stdout) {
 #if LLDB_ENABLE_LIBEDIT
   if (m_editline_up) {
+    std::lock_guard<std::mutex> guard(m_output_mutex);
     lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
     m_editline_up->PrintAsync(stream.get(), s, len);
   } else
 #endif
   {
 #ifdef _WIN32
     const char *prompt = GetPrompt();
     if (prompt) {

lldb/source/Host/common/Editline.cpp

 
 Editline *Editline::InstanceFor(EditLine *editline) {
   Editline *editor;
   el_get(editline, EL_CLIENTDATA, &editor);
   return editor;
 }
 
 Editline::Editline(const char *editline_name, FILE *input_file,
-                   FILE *output_file, FILE *error_file, bool color_prompts)
+                   FILE *output_file, FILE *error_file,
+                   std::mutex &output_mutex, bool color_prompts)
     : m_editor_status(EditorStatus::Complete), m_color_prompts(color_prompts),
       m_input_file(input_file), m_output_file(output_file),
-      m_error_file(error_file), m_input_connection(fileno(input_file), false) {
+      m_error_file(error_file), m_input_connection(fileno(input_file), false),
+      m_output_mutex(output_mutex) {
   // Get a shared history instance
   m_editor_name = (editline_name == nullptr) ? "lldb-tmp" : editline_name;
   m_history_sp = EditlineHistory::GetHistory(m_editor_name);
 
 #ifdef USE_SETUPTERM_WORKAROUND
   if (m_output_file) {
     const int term_fd = fileno(m_output_file);
     if (term_fd != -1) {

lldb/source/Interpreter/CommandInterpreter.cpp

 
 bool CommandInterpreter::WasInterrupted() const {
   bool was_interrupted =
       (m_command_state == CommandHandlingState::eInterrupted);
   lldbassert(!was_interrupted || m_iohandler_nesting_level > 0);
   return was_interrupted;
 }
 
-void CommandInterpreter::PrintCommandOutput(Stream &stream,
-                                            llvm::StringRef str) {
+void CommandInterpreter::PrintCommandOutput(IOHandler &io_handler,
+                                            llvm::StringRef str,
+                                            bool is_stdout) {
+
+  lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
+                                        : io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
   const char *data = str.data();
   size_t size = str.size();
   while (size > 0 && !WasInterrupted()) {
     size_t chunk_size = 0;
     for (; chunk_size < size; ++chunk_size) {
       lldbassert(data[chunk_size] != '\0');
       if (data[chunk_size] == '\n') {
         ++chunk_size;
         break;
       }
     }
-    chunk_size = stream.Write(data, chunk_size);
+    {
+      std::lock_guard<std::mutex> guard(io_handler.GetOutputMutex());
+      chunk_size = stream->Write(data, chunk_size);
+    }
     lldbassert(size >= chunk_size);
     data += chunk_size;
     size -= chunk_size;
   }
-  if (size > 0) {
-    stream.Printf("\n... Interrupted.\n");
-  }
-  stream.Flush();
+
+  std::lock_guard<std::mutex> guard(io_handler.GetOutputMutex());
+  if (size > 0)
+    stream->Printf("\n... Interrupted.\n");
+  stream->Flush();
 }
 
 bool CommandInterpreter::EchoCommandNonInteractive(
     llvm::StringRef line, const Flags &io_handler_flags) const {
   if (!io_handler_flags.Test(eHandleCommandFlagEchoCommand))
     return false;
 
   llvm::StringRef command = line.trim();

     // previous command and cause any errors to occur (like redefining an
     // alias, get an error and stop parsing the commands file).
     if (line.empty())
       return;
 
     // When using a non-interactive file handle (like when sourcing commands
     // from a file) we need to echo the command out so we don't just see the
     // command output and no command...
-    if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
+    if (EchoCommandNonInteractive(line, io_handler.GetFlags())) {
+      std::lock_guard<std::mutex> guard(io_handler.GetOutputMutex());
       io_handler.GetOutputStreamFileSP()->Printf(
           "%s%s\n", io_handler.GetPrompt(), line.c_str());
+    }
   }
 
   StartHandlingCommand();
 
   OverrideExecutionContext(m_debugger.GetSelectedExecutionContext());
   auto finalize = llvm::make_scope_exit([this]() {
     RestoreExecutionContext();
   });
 
   lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
   // Now emit the command output text from the command we just executed
   if ((result.Succeeded() &&
        io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) ||
       io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors)) {
     // Display any STDOUT/STDERR _prior_ to emitting the command result text
     GetProcessOutput();
 
     if (!result.GetImmediateOutputStream()) {
       llvm::StringRef output = result.GetOutputData();
-      PrintCommandOutput(*io_handler.GetOutputStreamFileSP(), output);
+      PrintCommandOutput(io_handler, output, true);
     }
 
     // Now emit the command error text from the command we just executed
     if (!result.GetImmediateErrorStream()) {
       llvm::StringRef error = result.GetErrorData();
-      PrintCommandOutput(*io_handler.GetErrorStreamFileSP(), error);
+      PrintCommandOutput(io_handler, error, false);
     }
   }
 
   FinishHandlingCommand();
 
   switch (result.GetStatus()) {
   case eReturnStatusInvalid:
   case eReturnStatusSuccessFinishNoResult:

lldb/unittests/Editline/EditlineTest.cpp

bool GetLines(lldb_private::StringList &lines, bool &interrupted,
                 size_t timeout_millis);
 
   void ConsumeAllOutput();
 
 private:
   bool IsInputComplete(lldb_private::Editline *editline,
                        lldb_private::StringList &lines);
 
+  std::mutex output_mutex;
   std::unique_ptr<lldb_private::Editline> _editline_sp;
 
   PseudoTerminal _pty;
   int _pty_primary_fd;
   int _pty_secondary_fd;
 
   std::unique_ptr<FilePointer> _el_secondary_file;
 };
EditlineAdapter::EditlineAdapter()
   _el_secondary_file.reset(new FilePointer(fdopen(_pty_secondary_fd, "rw")));
   EXPECT_FALSE(nullptr == *_el_secondary_file);
   if (*_el_secondary_file == nullptr)
     return;
 
   // Create an Editline instance.
   _editline_sp.reset(new lldb_private::Editline(
       "gtest editor", *_el_secondary_file, *_el_secondary_file,
-      *_el_secondary_file, false));
+      *_el_secondary_file, output_mutex, false));
   _editline_sp->SetPrompt("> ");
 
   // Hookup our input complete callback.
   auto input_complete_cb = [this](Editline *editline, StringList &lines) {
     return this->IsInputComplete(editline, lines);
   };
   _editline_sp->SetIsInputCompleteCallback(input_complete_cb);
 }