Index: include/lldb/Core/Debugger.h =================================================================== --- include/lldb/Core/Debugger.h +++ include/lldb/Core/Debugger.h @@ -14,6 +14,7 @@ #include // C++ Includes +#include #include #include @@ -204,6 +205,64 @@ bool CheckTopIOHandlerTypes(IOHandler::Type top_type, IOHandler::Type second_top_type); + //------------------------------------------------------------------ + /// Guard class that delays the output printing from the given debugger + /// until the end of the scope. Useful if you aquire any IOHandler locks + /// in the current scope and also call code that might print via an IOHandler + /// (which could lead to deadlocks). + /// + /// This guard can be used in nested scopes. Multiple guards on the + /// same debugger behave the same as if only the top-most guard + /// requested to delay the messages. + /// + /// If a nullptr is passed as a debugger object, this guard will perform no + /// actions. + /// + /// @see EnableDelayedPrinting() + /// @see TryFlushingDelayedMessages() + /// @see DisableMessageDelayScope + //------------------------------------------------------------------ + struct MessageDelayScope { + Debugger *m_debugger; + explicit MessageDelayScope(Debugger *d) : m_debugger(d) { + if (m_debugger) + m_debugger->EnableDelayedPrinting(); + } + ~MessageDelayScope() { + if (m_debugger) + m_debugger->TryFlushingDelayedMessages(); + } + DISALLOW_COPY_AND_ASSIGN(MessageDelayScope); + }; + + //------------------------------------------------------------------ + /// Guard class that reverts the effect of one existing MessageDelayScope + /// during its lifetime. This guard can only be instantiated on a debugger + /// with at least one active MessageDelayScope. If a nullptr is passed as a + /// debugger, this guard will perform no actions. + /// + /// @note This class is intended to make existing code compatible with + /// MessageDelayScope and shouldn't be used in new code if possible. + /// + /// @see MessageDelayScope + //------------------------------------------------------------------ + struct DisableMessageDelayScope { + Debugger *m_debugger; + // TODO: This constructor should not take a debugger object, but rather an + // existing MessageDelayScope to prevent using it in a spot where no active + // MessageDelayScope exists. However, we currently don't have access to the + // current MessageDelayScope object in all places where we need this. + explicit DisableMessageDelayScope(Debugger *d) : m_debugger(d) { + if (m_debugger) + m_debugger->TryFlushingDelayedMessages(); + } + ~DisableMessageDelayScope() { + if (m_debugger) + m_debugger->EnableDelayedPrinting(); + } + DISALLOW_COPY_AND_ASSIGN(DisableMessageDelayScope); + }; + void PrintAsync(const char *s, size_t len, bool is_stdout); ConstString GetTopIOHandlerControlSequence(char ch); @@ -417,6 +476,54 @@ // object Debugger(lldb::LogOutputCallback m_log_callback, void *baton); + //------------------------------------------------------------------ + /// A message sent via PrintAsync that we store to delay the actual + /// call until later. + //------------------------------------------------------------------ + struct DelayedMessage { + DelayedMessage(std::string data, bool for_stdout) + : m_data(data), m_for_stdout(for_stdout) {} + std::string m_data; + bool m_for_stdout; + }; + + //------------------------------------------------------------------ + /// Starts delaying any calls to PrintAsync until a matching call + /// to FlushDelayedMessages occurs. This function should only be + /// called before a matching call to FlushDelayedMessages. + /// + /// This method is intentionally private. Use MessageDelayScope to + /// call this method (and the matching FlushDelayedMessage()) method + /// below. + //------------------------------------------------------------------ + void EnableDelayedPrinting() { + std::lock_guard guard(m_delayed_output_mutex); + ++m_delayed_output_counter; + } + + //------------------------------------------------------------------ + /// Tries to flush any delayed messages to PrintAsync. This might + /// not be possible if it was requested by multiple users to + /// delay messages and not all have given consent at this point for + /// forwarding the messages. + //------------------------------------------------------------------ + void TryFlushingDelayedMessages(); + + /// This mutex protects *only* m_async_buffer and m_should_buffer_async. + std::mutex m_delayed_output_mutex; + /// The list of delayed messages in the order in which they arrived. + /// @note Protected by m_delayed_output_mutex. + std::vector m_delayed_output; + /// This counter keeps track of how many users have requested that we delay + /// the forwarding of calls to PrintAsync to the IOHandlers. If the counter + /// is 0, then it's safe to directly forward all calls to PrintAsync to the + /// IOHandlers. If the counter is *not* 0, then all calls to PrintAsync + /// should be cached until the counter is 0 again. + /// @see EnableDelayedPrinting() + /// @see TryFlushingDelayedMessages(). + /// @note Protected by m_delayed_output_mutex. + unsigned m_delayed_output_counter = 0; + DISALLOW_COPY_AND_ASSIGN(Debugger); }; Index: include/lldb/Host/Editline.h =================================================================== --- include/lldb/Host/Editline.h +++ include/lldb/Host/Editline.h @@ -58,6 +58,7 @@ #include "lldb/Utility/FileSpec.h" namespace lldb_private { +class Debugger; namespace line_editor { // type alias's to help manage 8 bit and wide character versions of libedit @@ -146,7 +147,8 @@ class Editline { public: Editline(const char *editor_name, FILE *input_file, FILE *output_file, - FILE *error_file, bool color_prompts); + FILE *error_file, bool color_prompts, + lldb_private::Debugger *debugger); ~Editline(); @@ -359,6 +361,9 @@ CompleteCallbackType m_completion_callback = nullptr; void *m_completion_callback_baton = nullptr; + /// The debugger that is using this Editline instance for input/output. May + /// be a nullptr if there is no debugger using this instance. + lldb_private::Debugger *m_debugger = nullptr; std::mutex m_output_mutex; }; } Index: source/Core/Debugger.cpp =================================================================== --- source/Core/Debugger.cpp +++ source/Core/Debugger.cpp @@ -797,6 +797,29 @@ SetUseColor(false); } +void Debugger::TryFlushingDelayedMessages() { + // We copy the buffer here. We do *not* want to call PrintAsync (or + // anything else) while we own this lock. + std::vector buffer_to_send; + { + std::lock_guard guard(m_delayed_output_mutex); + buffer_to_send = std::move(m_delayed_output); + m_delayed_output.clear(); + // It's an error if we call this method before EnableDelayedPrinting. + assert(m_delayed_output_counter > 0); + --m_delayed_output_counter; + // 0 means that all users we have have consented that we can now forward + // our delayed messages to the IOHandlers. Other values mean we that can't + // forward the delayed messages yet. + if (m_delayed_output_counter != 0) + return; + } + // Forward all cached messages to the IOHandlers. + for (const auto &msg : buffer_to_send) { + PrintAsync(msg.m_data.data(), msg.m_data.size(), msg.m_for_stdout); + } +} + Debugger::~Debugger() { Clear(); } void Debugger::Clear() { @@ -985,6 +1008,16 @@ } void Debugger::PrintAsync(const char *s, size_t len, bool is_stdout) { + // We check if any user requested to delay output to a later time + // (which is designated by m_delayed_output_counter being not 0). + { + std::lock_guard guard(m_delayed_output_mutex); + if (m_delayed_output_counter != 0) { + // We want to delay messages, so push them to the buffer. + m_delayed_output.emplace_back(std::string(s, len), is_stdout); + return; + } + } lldb::StreamFileSP stream = is_stdout ? GetOutputFile() : GetErrorFile(); m_input_reader_stack.PrintAsync(stream.get(), s, len); } Index: source/Core/IOHandler.cpp =================================================================== --- source/Core/IOHandler.cpp +++ source/Core/IOHandler.cpp @@ -313,7 +313,7 @@ if (use_editline) { m_editline_ap.reset(new Editline(editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(), - m_color_prompts)); + m_color_prompts, nullptr)); m_editline_ap->SetIsInputCompleteCallback(IsInputCompleteCallback, this); m_editline_ap->SetAutoCompleteCallback(AutoCompleteCallback, this); // See if the delegate supports fixing indentation Index: source/Host/common/Editline.cpp =================================================================== --- source/Host/common/Editline.cpp +++ source/Host/common/Editline.cpp @@ -11,6 +11,7 @@ #include #include +#include "lldb/Core/Debugger.h" #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/Editline.h" #include "lldb/Host/Host.h" @@ -507,9 +508,15 @@ // indefinitely. This gives a chance for someone to interrupt us. After // Read returns, immediately lock the mutex again and check if we were // interrupted. + int read_count; m_output_mutex.unlock(); - int read_count = m_input_connection.Read(&ch, 1, llvm::None, status, NULL); + { + // We also disable the message delaying while we have the mutex unlocked. + lldb_private::Debugger::DisableMessageDelayScope guard(m_debugger); + read_count = m_input_connection.Read(&ch, 1, llvm::None, status, NULL); + } m_output_mutex.lock(); + if (m_editor_status == EditorStatus::Interrupted) { while (read_count > 0 && status == lldb::eConnectionStatusSuccess) read_count = m_input_connection.Read(&ch, 1, llvm::None, status, NULL); @@ -1128,10 +1135,12 @@ } Editline::Editline(const char *editline_name, FILE *input_file, - FILE *output_file, FILE *error_file, bool color_prompts) + FILE *output_file, FILE *error_file, bool color_prompts, + Debugger *debugger) : 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_debugger(debugger) { // Get a shared history instance m_editor_name = (editline_name == nullptr) ? "lldb-tmp" : editline_name; m_history_sp = EditlineHistory::GetHistory(m_editor_name); @@ -1264,6 +1273,12 @@ m_input_lines = std::vector(); m_input_lines.insert(m_input_lines.begin(), EditLineConstString("")); + // While we own the output mutex, we delay all messages that are printed via + // PrintAsync until we release the mutex again. This prevents dead locks that + // are caused when someone calls PrintAsync (which also needs to aquire + // the output mutex). Note that Editline::GetCharacter unlocks the guarded + // mutex from a nested call and also temporarily disables the message delay. + Debugger::MessageDelayScope msg_guard(m_debugger); std::lock_guard guard(m_output_mutex); lldbassert(m_editor_status != EditorStatus::Editing); @@ -1309,6 +1324,9 @@ m_input_lines = std::vector(); m_input_lines.insert(m_input_lines.begin(), EditLineConstString("")); + // We delay all message printing until we release the output mutex. See + // Editline::GetLine for a more detailed explanation. + Debugger::MessageDelayScope msg_guard(m_debugger); std::lock_guard guard(m_output_mutex); // Begin the line editing loop DisplayInput(); Index: unittests/Editline/EditlineTest.cpp =================================================================== --- unittests/Editline/EditlineTest.cpp +++ unittests/Editline/EditlineTest.cpp @@ -123,9 +123,9 @@ return; // Create an Editline instance. - _editline_sp.reset(new lldb_private::Editline("gtest editor", *_el_slave_file, - *_el_slave_file, - *_el_slave_file, false)); + _editline_sp.reset(new lldb_private::Editline( + "gtest editor", *_el_slave_file, *_el_slave_file, *_el_slave_file, false, + nullptr)); _editline_sp->SetPrompt("> "); // Hookup our input complete callback.