[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push the ProcessIO handler.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: jingham.
Herald added a subscriber: abidh.

https://reviews.llvm.org/D48465 is currently blocked by the fact that tab-completing the first expression is deadlocking LLDB.

The reason for this deadlock is that when we push the ProcessIO handler for reading the Objective-C runtime
information from the executable (which is triggerd when we parse the an expression for the first time_,
the IOHandler can't be pushed as the Editline::Cancel method is deadlocking.

The deadlock in Editline is coming from the m_output_mutex, which is locked before we go into tab completion.
Even without this lock, calling Cancel on Editline will mean that Editline cleans up behind itself and deletes the
current user-input, which is screws up the console when we are tab-completing at the same time.

I think for now the most reasonable way of fixing this is to just not call Cancel on the current IOHandler when we
push the ProcessIO handler. The ProcessIO handler is anyway always temporary, so not cleaning up for the current
IOHandler for the short duration while the non-interactive ProcessIO handler is in charge doesn't sound too bad.

As we can't really write unit tests for IOHandler itself (due to the hard dependency on an initialized Debugger including
all its global state) and Editline completion is currently also not really testable in an automatic fashion, the test for this has to be that the expression command completion in https://reviews.llvm.org/D48465 doesn't fail the first time.

I'll provide a proper unit test for this once I got around and made the IOHandler code testable, but for now unblocking
https://reviews.llvm.org/D48465 is more critical.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50912

Files:
  source/Core/Debugger.cpp


Index: source/Core/Debugger.cpp
===================================================================
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -1098,7 +1098,11 @@
   // this new input reader take over
   if (top_reader_sp) {
     top_reader_sp->Deactivate();
-    top_reader_sp->Cancel();
+    // For the ProcessIO handler we don't cancel the current IOHandler. The
+    // current IOHandler shouldn't be influenced by the fact that the temporary
+    // ProcessIO handler has hijacked the IO.
+    if (reader_sp->GetType() != IOHandler::Type::ProcessIO)
+      top_reader_sp->Cancel();
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50912.161286.patch
Type: text/x-patch
Size: 615 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180817/c2977520/attachment.bin>