[Lldb-commits] [PATCH] D48463: Prevent dead locking when calling PrintAsync

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked 2 inline comments as done.
teemperor added a comment.

@labath Thanks for the explanation! Well in theory we could poke more holes in our guard from some nested function, but this only fixes the deadlock symptom. PrintAsync would still be blocking whenever the mutex is hold by someone.



================
Comment at: source/Core/Debugger.cpp:988-1004
+  bool should_forward = false;
+  {
+    // 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<std::mutex> guard(m_delayed_output_mutex);
+    if (m_delayed_output_counter != 0) {
+      // We want to delay messages, so push them to the buffer.
----------------
clayborg wrote:
> Can this code become:
> 
> ```
> // 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<std::mutex> 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);
> ```
It can and did. Removed the same pattern from the TryFlushingDelayedMessages method.


================
Comment at: source/Core/IOHandler.cpp:358
+  // lldb code that will be called from here (possibly in another thread).
+  Debugger::MessageDelayScope buffer_scope(m_debugger);
+
----------------
clayborg wrote:
> So anytime we have a "(lldb)" prompt we won't be able to output something?
Yes, that was the unintended effect of this patch (which was caught by the test suite actually). I didn't see until after I submitted that we actually manually unlock the guarded mutex from a called function. This now has been fixed by doing something like the unlock/lock code with the DisableMessageDelayScope in `EditLine::GetCharacter`.


https://reviews.llvm.org/D48463