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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to