[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D72748#1830638 , @labath wrote:

> In D72748#1829956 , @JDevlieghere 
> wrote:
>
> > In D72748#1823945 , @labath wrote:
> >
> > > I didn't actually try it but I am pretty sure this will deadlock with 
> > > nested lldb command files (running `command source` from a file that is 
> > > itself being sourced). Changing the mutex to a recursive_mutex would fix 
> > > that, but I don't believe it would make this fully correct -- it would 
> > > just make it harder to demonstrate that it's wrong. OTOH, that may be the 
> > > best thing we can do in the current state of affairs.
> > >
> > > The thing I don't understand now is why do we even need this stack in the 
> > > first place. It seems like this could be handled by just running a new 
> > > iohandler "main loop" instead of pushing something. Take the "expr" 
> > > command for example. In the single-line mode it evaluates the expression 
> > > synchronously, but in a multi-line expression, it returns immediately 
> > > after pushing it's own IOHandler (which then gathers the expression and 
> > > calls back into the command to run it). I don't see why we couldn't 
> > > achieve the same thing by "running" the iohandler directly, instead of 
> > > pushing it to some stack and waiting for it to be executed at the top 
> > > level. The same thing could be said for the "script" command and various 
> > > other things which "hijack" the main (lldb) iohandler.
> >
> >
> > Isn't the problem that you can't be sure your IO handler pushes another one 
> > on top of the stack? I considered an alternative implementation, where the 
> > synchronous IO handlers has its own stack and everything that's pushed 
> > while it is executing ends up on that stack. It adds a lot of complexity 
> > and you still need to synchronize with the "main loop"
>
>
> Well.. in the way I as imagining things, there would be no stacks (at least 
> no explicit stacks), no pushing, and everything would execute in the 
> "synchronous" mode. So e.g., when we start up the main "(lldb)" loop, we just 
> take that iohandler, and run it until it says it's done. If the user types 
> "script", then we start another "main loop" with the python iohandler, but 
> the main loop can be completely oblivious to that -- as far as it is 
> concerned, its iohandler is still executing `CommandObjectScript::DoExecute`. 
> When the user exits the python prompt the control returns to the (lldb) 
> iohandler just like it would after any other "simple" command. So, 
> essentially, there's still some stacking involved, but it's not managed 
> explicitly -- it just comes out from the way the code is organized. 
> Similarly, the "breakpoint command add" could run an iohandler to collect the 
> breakpoint commands, "process continue" could run a loop to forward the 
> inferior stdio, etc.
>
> (The last bit is tricky because of ^C, and it means that we will still need 
> to have some global notion of the "active" or "top" iohandler, which is the 
> one that receives ^Cs, but still, I think that it should be possible to run 
> everything "synchronously" and I hope that would get us rid of a lot of 
> complexity.)


The pushing and popping is done because of how the command interpreter works. 
Think about typing:

  (lldb) script

We are in the IOHandler for the command interpreter for LLDB commands and 
running the code for the "script" command in 
CommandObjectScript::DoExecute(...), and now we need to switch to the python 
interpreter.

Before doing this we might need to remove the "(lldb)" prompt from the screen 
if had already been redisplayed. The idea was while in 
CommandObjectScript::DoExecute(...) we can push a new IOHandler for python and 
let the current function exit. There might be some locks preventing multiple 
commands from executing at the same time, so best to avoid this if this is the 
case. Then when we return from CommandObjectScript::DoExecute(...) we can let 
the IOHandler stack do what it needs to do and switch to the python interpreter.

This flow also allows the "gui" command to do any curses setup and teardown at 
the right times as the IOHandler is pushed and popped.

When the previous IOHandler becomes active again, it can resume where it left 
off, like curses can clear the screen and re-display anything it needs to. So 
the stack does serve a purpose, but I am sure this can be done in another way 
and happy to try a new strategy.

One complexity you won't see here is how the REPL and LLDB prompt can switch 
back and forth. From the Swift REPL, you can drop into the lldb prompt and 
back, and for that we have some special handling that allows the two to switch 
without creating a whole new IOHandler each time they switch.

Also think about a file that is sourced with "command source" and where it it 
contains:

  target stop-hook add
  bt
  fr 

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D72748#1829956 , @JDevlieghere 
wrote:

> In D72748#1823945 , @labath wrote:
>
> > I didn't actually try it but I am pretty sure this will deadlock with 
> > nested lldb command files (running `command source` from a file that is 
> > itself being sourced). Changing the mutex to a recursive_mutex would fix 
> > that, but I don't believe it would make this fully correct -- it would just 
> > make it harder to demonstrate that it's wrong. OTOH, that may be the best 
> > thing we can do in the current state of affairs.
> >
> > The thing I don't understand now is why do we even need this stack in the 
> > first place. It seems like this could be handled by just running a new 
> > iohandler "main loop" instead of pushing something. Take the "expr" command 
> > for example. In the single-line mode it evaluates the expression 
> > synchronously, but in a multi-line expression, it returns immediately after 
> > pushing it's own IOHandler (which then gathers the expression and calls 
> > back into the command to run it). I don't see why we couldn't achieve the 
> > same thing by "running" the iohandler directly, instead of pushing it to 
> > some stack and waiting for it to be executed at the top level. The same 
> > thing could be said for the "script" command and various other things which 
> > "hijack" the main (lldb) iohandler.
>
>
> Isn't the problem that you can't be sure your IO handler pushes another one 
> on top of the stack? I considered an alternative implementation, where the 
> synchronous IO handlers has its own stack and everything that's pushed while 
> it is executing ends up on that stack. It adds a lot of complexity and you 
> still need to synchronize with the "main loop"


Well.. in the way I as imagining things, there would be no stacks (at least no 
explicit stacks), no pushing, and everything would execute in the "synchronous" 
mode. So e.g., when we start up the main "(lldb)" loop, we just take that 
iohandler, and run it until it says it's done. If the user types "script", then 
we start another "main loop" with the python iohandler, but the main loop can 
be completely oblivious to that -- as far as it is concerned, its iohandler is 
still executing `CommandObjectScript::DoExecute`. When the user exits the 
python prompt the control returns to the (lldb) iohandler just like it would 
after any other "simple" command. So, essentially, there's still some stacking 
involved, but it's not managed explicitly -- it just comes out from the way the 
code is organized. Similarly, the "breakpoint command add" could run an 
iohandler to collect the breakpoint commands, "process continue" could run a 
loop to forward the inferior stdio, etc.

(The last bit is tricky because of ^C, and it means that we will still need to 
have some global notion of the "active" or "top" iohandler, which is the one 
that receives ^Cs, but still, I think that it should be possible to run 
everything "synchronously" and I hope that would get us rid of a lot of 
complexity.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG04de24e690d3: [lldb/IOHandler] Improve synchronization 
between IO handlers. (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D72748?vs=238886=239178#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -895,23 +895,62 @@
 }
 
 void Debugger::RunIOHandlers() {
+  IOHandlerSP reader_sp = m_io_handler_stack.Top();
   while (true) {
-IOHandlerSP reader_sp(m_io_handler_stack.Top());
 if (!reader_sp)
   break;
 
 reader_sp->Run();
+{
+  std::lock_guard guard(
+  m_io_handler_synchronous_mutex);
+
+  // Remove all input readers that are done from the top of the stack
+  while (true) {
+IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
+if (top_reader_sp && top_reader_sp->GetIsDone())
+  PopIOHandler(top_reader_sp);
+else
+  break;
+  }
+  reader_sp = m_io_handler_stack.Top();
+}
+  }
+  ClearIOHandlers();
+}
+
+void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
+  std::lock_guard guard(m_io_handler_synchronous_mutex);
+
+  PushIOHandler(reader_sp);
+  IOHandlerSP top_reader_sp = reader_sp;
 
-// Remove all input readers that are done from the top of the stack
+  while (top_reader_sp) {
+if (!top_reader_sp)
+  break;
+
+top_reader_sp->Run();
+
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get()) {
+  if (PopIOHandler(reader_sp))
+break;
+}
+
+// If we pushed new IO handlers, pop them if they're done or restart the
+// loop to run them if they're not.
 while (true) {
-  IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
+  top_reader_sp = m_io_handler_stack.Top();
+  if (top_reader_sp && top_reader_sp->GetIsDone()) {
 PopIOHandler(top_reader_sp);
-  else
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get())
+  return;
+  } else {
 break;
+  }
 }
   }
-  ClearIOHandlers();
 }
 
 bool Debugger::IsTopIOHandler(const lldb::IOHandlerSP _sp) {
@@ -950,28 +989,6 @@
   PushIOHandler(reader_sp, cancel_top_handler);
 }
 
-void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
@@ -0,0 +1 @@
+script foo = 95126
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
@@ -0,0 +1,4 @@
+int main (int argc, char const *argv[])
+{
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
@@ -0,0 +1,35 @@
+"""
+Test completion in our IOHandlers.

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

In D72748#1823945 , @labath wrote:

> I didn't actually try it but I am pretty sure this will deadlock with nested 
> lldb command files (running `command source` from a file that is itself being 
> sourced). Changing the mutex to a recursive_mutex would fix that, but I don't 
> believe it would make this fully correct -- it would just make it harder to 
> demonstrate that it's wrong. OTOH, that may be the best thing we can do in 
> the current state of affairs.
>
> The thing I don't understand now is why do we even need this stack in the 
> first place. It seems like this could be handled by just running a new 
> iohandler "main loop" instead of pushing something. Take the "expr" command 
> for example. In the single-line mode it evaluates the expression 
> synchronously, but in a multi-line expression, it returns immediately after 
> pushing it's own IOHandler (which then gathers the expression and calls back 
> into the command to run it). I don't see why we couldn't achieve the same 
> thing by "running" the iohandler directly, instead of pushing it to some 
> stack and waiting for it to be executed at the top level. The same thing 
> could be said for the "script" command and various other things which 
> "hijack" the main (lldb) iohandler.


Isn't the problem that you can't be sure your IO handler pushes another one on 
top of the stack? I considered an alternative implementation, where the 
synchronous IO handlers has its own stack and everything that's pushed while it 
is executing ends up on that stack. It adds a lot of complexity and you still 
need to synchronize with the "main loop"




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py:40-42
+self.child.send("script print(foo)\n")
+self.expect_prompt()
+self.child.expect_exact("95126")

labath wrote:
> There's a ptrace version of `self.expect` to automate these things for you. 
> It probably won't work for the multiline "breakpoint command add" command, 
> but the rest could be something like `self.expect("script print(foo)", 
> substrs=["95126"])`
Cool, I didn't know, I'll refactor that before landing


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Let's give this a shot. I'm don't think this is fully right, but I also don't 
know how to improve that, exactly...




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py:15
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)

Btw, all pexpect tests get this automatically from the base class.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py:40-42
+self.child.send("script print(foo)\n")
+self.expect_prompt()
+self.child.expect_exact("95126")

There's a ptrace version of `self.expect` to automate these things for you. It 
probably won't work for the multiline "breakpoint command add" command, but the 
rest could be something like `self.expect("script print(foo)", 
substrs=["95126"])`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 238886.
JDevlieghere added a comment.

Add PExpect test


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -895,23 +895,62 @@
 }
 
 void Debugger::RunIOHandlers() {
+  IOHandlerSP reader_sp = m_io_handler_stack.Top();
   while (true) {
-IOHandlerSP reader_sp(m_io_handler_stack.Top());
 if (!reader_sp)
   break;
 
 reader_sp->Run();
+{
+  std::lock_guard guard(
+  m_io_handler_synchronous_mutex);
+
+  // Remove all input readers that are done from the top of the stack
+  while (true) {
+IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
+if (top_reader_sp && top_reader_sp->GetIsDone())
+  PopIOHandler(top_reader_sp);
+else
+  break;
+  }
+  reader_sp = m_io_handler_stack.Top();
+}
+  }
+  ClearIOHandlers();
+}
+
+void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
+  std::lock_guard guard(m_io_handler_synchronous_mutex);
+
+  PushIOHandler(reader_sp);
+  IOHandlerSP top_reader_sp = reader_sp;
 
-// Remove all input readers that are done from the top of the stack
+  while (top_reader_sp) {
+if (!top_reader_sp)
+  break;
+
+top_reader_sp->Run();
+
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get()) {
+  if (PopIOHandler(reader_sp))
+break;
+}
+
+// If we pushed new IO handlers, pop them if they're done or restart the
+// loop to run them if they're not.
 while (true) {
-  IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
+  top_reader_sp = m_io_handler_stack.Top();
+  if (top_reader_sp && top_reader_sp->GetIsDone()) {
 PopIOHandler(top_reader_sp);
-  else
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get())
+  return;
+  } else {
 break;
+  }
 }
   }
-  ClearIOHandlers();
 }
 
 bool Debugger::IsTopIOHandler(const lldb::IOHandlerSP _sp) {
@@ -950,28 +989,6 @@
   PushIOHandler(reader_sp, cancel_top_handler);
 }
 
-void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/source.lldb
@@ -0,0 +1 @@
+script foo = 95126
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/main.c
@@ -0,0 +1,4 @@
+int main (int argc, char const *argv[])
+{
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
@@ -0,0 +1,43 @@
+"""
+Test completion in our IOHandlers.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+
+class BreakpointCallbackCommandSource(PExpectTest):

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Not super ideal, but not too bad either. Not clicking accept yet because of the 
missing test case...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 238684.
JDevlieghere added a comment.

Use recursive mutex


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -895,23 +895,62 @@
 }
 
 void Debugger::RunIOHandlers() {
+  IOHandlerSP reader_sp = m_io_handler_stack.Top();
   while (true) {
-IOHandlerSP reader_sp(m_io_handler_stack.Top());
 if (!reader_sp)
   break;
 
 reader_sp->Run();
+{
+  std::lock_guard guard(
+  m_io_handler_synchronous_mutex);
+
+  // Remove all input readers that are done from the top of the stack
+  while (true) {
+IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
+if (top_reader_sp && top_reader_sp->GetIsDone())
+  PopIOHandler(top_reader_sp);
+else
+  break;
+  }
+  reader_sp = m_io_handler_stack.Top();
+}
+  }
+  ClearIOHandlers();
+}
+
+void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
+  std::lock_guard guard(m_io_handler_synchronous_mutex);
+
+  PushIOHandler(reader_sp);
+  IOHandlerSP top_reader_sp = reader_sp;
 
-// Remove all input readers that are done from the top of the stack
+  while (top_reader_sp) {
+if (!top_reader_sp)
+  break;
+
+top_reader_sp->Run();
+
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get()) {
+  if (PopIOHandler(reader_sp))
+break;
+}
+
+// If we pushed new IO handlers, pop them if they're done or restart the
+// loop to run them if they're not.
 while (true) {
-  IOHandlerSP top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
+  top_reader_sp = m_io_handler_stack.Top();
+  if (top_reader_sp && top_reader_sp->GetIsDone()) {
 PopIOHandler(top_reader_sp);
-  else
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get())
+  return;
+  } else {
 break;
+  }
 }
   }
-  ClearIOHandlers();
 }
 
 bool Debugger::IsTopIOHandler(const lldb::IOHandlerSP _sp) {
@@ -950,28 +989,6 @@
   PushIOHandler(reader_sp, cancel_top_handler);
 }
 
-void Debugger::RunIOHandlerSync(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_io_handler_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -410,6 +410,8 @@
   m_script_interpreters;
 
   IOHandlerStack m_io_handler_stack;
+  std::recursive_mutex m_io_handler_synchronous_mutex;
+
   llvm::StringMap> m_log_streams;
   std::shared_ptr m_log_callback_stream_sp;
   ConstString m_instance_name;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I didn't actually try it but I am pretty sure this will deadlock with nested 
lldb command files (running `command source` from a file that is itself being 
sourced). Changing the mutex to a recursive_mutex would fix that, but I don't 
believe it would make this fully correct -- it would just make it harder to 
demonstrate that it's wrong. OTOH, that may be the best thing we can do in the 
current state of affairs.

The thing I don't understand now is why do we even need this stack in the first 
place. It seems like this could be handled by just running a new iohandler 
"main loop" instead of pushing something. Take the "expr" command for example. 
In the single-line mode it evaluates the expression synchronously, but in a 
multi-line expression, it returns immediately after pushing it's own IOHandler 
(which then gathers the expression and calls back into the command to run it). 
I don't see why we couldn't achieve the same thing by "running" the iohandler 
directly, instead of pushing it to some stack and waiting for it to be executed 
at the top level. The same thing could be said for the "script" command and 
various other things which "hijack" the main (lldb) iohandler.

Greg, am I missing something?




Comment at: lldb/include/lldb/Core/Debugger.h:406
   IOHandlerStack m_input_reader_stack;
+  std::mutex m_io_hanlder_synchronous_mutex;
+

typo


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 238358.
JDevlieghere added a comment.

Synchronize between `RunIOHandler` and `ExecuteIOHandlers` using a mutex. When 
running an IO handler synchronously, block `ExecuteIOHandlers` until we're 
finished.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -895,23 +895,62 @@
 }
 
 void Debugger::ExecuteIOHandlers() {
+  IOHandlerSP reader_sp = m_input_reader_stack.Top();
   while (true) {
-IOHandlerSP reader_sp(m_input_reader_stack.Top());
 if (!reader_sp)
   break;
 
 reader_sp->Run();
 
-// Remove all input readers that are done from the top of the stack
+{
+  std::lock_guard guard(m_io_hanlder_synchronous_mutex);
+
+  // Remove all input readers that are done from the top of the stack
+  while (true) {
+IOHandlerSP top_reader_sp = m_input_reader_stack.Top();
+if (top_reader_sp && top_reader_sp->GetIsDone())
+  PopIOHandler(top_reader_sp);
+else
+  break;
+  }
+  reader_sp = m_input_reader_stack.Top();
+}
+  }
+  ClearIOHandlers();
+}
+
+void Debugger::RunIOHandler(const IOHandlerSP _sp) {
+  std::lock_guard guard(m_io_hanlder_synchronous_mutex);
+
+  PushIOHandler(reader_sp);
+  IOHandlerSP top_reader_sp = reader_sp;
+
+  while (top_reader_sp) {
+if (!top_reader_sp)
+  break;
+
+top_reader_sp->Run();
+
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get()) {
+  if (PopIOHandler(reader_sp))
+break;
+}
+
+// If we pushed new IO handlers, pop them if they're done or restart the
+// loop to run them if they're not.
 while (true) {
-  IOHandlerSP top_reader_sp = m_input_reader_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
+  top_reader_sp = m_input_reader_stack.Top();
+  if (top_reader_sp && top_reader_sp->GetIsDone()) {
 PopIOHandler(top_reader_sp);
-  else
+// Don't unwind past the starting point.
+if (top_reader_sp.get() == reader_sp.get())
+  return;
+  } else {
 break;
+  }
 }
   }
-  ClearIOHandlers();
 }
 
 bool Debugger::IsTopIOHandler(const lldb::IOHandlerSP _sp) {
@@ -941,28 +980,6 @@
   return m_input_reader_stack.GetTopIOHandlerHelpPrologue();
 }
 
-void Debugger::RunIOHandler(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_input_reader_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -403,6 +403,8 @@
   m_script_interpreters;
 
   IOHandlerStack m_input_reader_stack;
+  std::mutex m_io_hanlder_synchronous_mutex;
+
   llvm::StringMap> m_log_streams;
   std::shared_ptr m_log_callback_stream_sp;
   ConstString m_instance_name;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere planned changes to this revision.
JDevlieghere added a comment.

I'm working on a different approach that should address al the concerns raised 
so far.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D72748#1822443 , @clayborg wrote:

> So I did a bunch of original IOHandler. And there are many complex cases for 
> sure. One thing to be aware of is that if you won't use editline() and we 
> call fgets() in the default implementation, there is no way to cancel this 
> IIRC. So it might be worth trying this without editline support to make sure 
> things don't deadlock. If the test suite is happy, then this looks worth 
> trying, though with all the complexities I don't think we can guarantee this 
> doesn't cause issues in some unexpected way. The main things that worry me 
> are:
>
> - REPL issues since the REPL and (lldb) prompt switch between themselves in a 
> tricky way where they swap IOHandlers
> - running from python script directly when no IOHandlers are pushed because 
> no command interpreter is being run and all the fallout from the cases 
> (HandleCommand that results in a python breakpoint callback etc)
> - the process IO handler that does STDIO for a process
> - when no editline, we use fgets() that can't be canceled


The fgets part is problematic in other ways.  For instance, Python based 
commands in stop hooks work in command line lldb, but in Xcode which doesn't 
use editline, when we go to fflush the I/O channel on switching to the Python 
interpreter, fflush deadlocks against the lock held by fgets.  I wonder if we 
should just stop using fgets and use select everywhere we can, as it doesn't 
have this problem.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So I did a bunch of original IOHandler. And there are many complex cases for 
sure. One thing to be aware of is that if you won't use editline() and we call 
fgets() in the default implementation, there is no way to cancel this IIRC. So 
it might be worth trying this without editline support to make sure things 
don't deadlock. If the test suite is happy, then this looks worth trying, 
though with all the complexities I don't think we can guarantee this doesn't 
cause issues in some unexpected way. The main things that worry me are:

- REPL issues since the REPL and (lldb) prompt switch between themselves in a 
tricky way where they swap IOHandlers
- running from python script directly when no IOHandlers are pushed because no 
command interpreter is being run and all the fallout from the cases 
(HandleCommand that results in a python breakpoint callback etc)
- the process IO handler that does STDIO for a process
- when no editline, we use fgets() that can't be canceled


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Though I have messed with IOHandlers in the past, I have successfully 
suppressed most of the memories of it. I think I have a rough understanding of 
what the bug is, but I don't understand the solution yet.

With this patch, what does guarantee that the IOHandler for the `"command 
source -s true ./test.lldb"` thingy completes before the breakpoint callback is 
finished (I assume that the intention is for this to be executed synchronously)?

I don't know if this matters, but another detail to be aware of is that the 
IOHandler stack will be different if driving lldb through python (without 
calling SBDebugger::RunCommandInterpreter). In this case there won't be a stdio 
editline handler sitting at the bottom of the stack.




Comment at: lldb/source/Core/Debugger.cpp:1020
+  // they aren't running yet.
+  if (asynchronous_if_needed && !m_synchronous_reader_lock.owns_lock()) {
+ExecuteIOHandlers();

This looks very clever, but it can still be racy if someone calls 
ExecuteIOHandlers concurrently to the `owns_lock` check...

A better way to achieve this (if I understand correctly what you are trying to 
achieve) would be to have a `ExecuteIOHandlersIfNeeded` function which does 
something like
```
std::unique_lock lock(m_synchronous_reader_mutex, std::try_lock);
if (lock)
  ReallyExecuteIOHandlers(); // No locking in here
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 238172.
JDevlieghere added a comment.

Rebase on top of NFC change


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1854,7 +1854,7 @@
   IOHandlerConfirm *confirm =
   new IOHandlerConfirm(m_debugger, message, default_answer);
   IOHandlerSP io_handler_sp(confirm);
-  m_debugger.RunIOHandler(io_handler_sp);
+  m_debugger.PushIOHandler(io_handler_sp);
   return confirm->GetResponse();
 }
 
@@ -2477,7 +2477,11 @@
 
   m_command_source_depth++;
 
-  debugger.RunIOHandler(io_handler_sp);
+  const bool cancel_top_handler = true;
+  const bool asynchronous_if_needed = true;
+  debugger.PushIOHandler(io_handler_sp, cancel_top_handler,
+ asynchronous_if_needed);
+
   if (!m_command_source_flags.empty())
 m_command_source_flags.pop_back();
   m_command_source_depth--;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -708,9 +708,10 @@
   m_source_manager_up(), m_source_file_cache(),
   m_command_interpreter_up(
   std::make_unique(*this, false)),
-  m_input_reader_stack(), m_instance_name(), m_loaded_plugins(),
-  m_event_handler_thread(), m_io_handler_thread(),
-  m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
+  m_input_reader_stack(),
+  m_synchronous_reader_lock(m_synchronous_reader_mutex, std::defer_lock),
+  m_instance_name(), m_loaded_plugins(), m_event_handler_thread(),
+  m_io_handler_thread(), m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
   m_forward_listener_sp(), m_clear_once() {
   char instance_cstr[256];
   snprintf(instance_cstr, sizeof(instance_cstr), "debugger_%d", (int)GetID());
@@ -895,6 +896,11 @@
 }
 
 void Debugger::ExecuteIOHandlers() {
+  // Prevent anyone from executing the IO handlers synchronously while they're
+  // running here.
+  std::lock_guard> guard(
+  m_synchronous_reader_lock);
+
   while (true) {
 IOHandlerSP reader_sp(m_input_reader_stack.Top());
 if (!reader_sp)
@@ -941,28 +947,6 @@
   return m_input_reader_stack.GetTopIOHandlerHelpPrologue();
 }
 
-void Debugger::RunIOHandler(const IOHandlerSP _sp) {
-  PushIOHandler(reader_sp);
-
-  IOHandlerSP top_reader_sp = reader_sp;
-  while (top_reader_sp) {
-top_reader_sp->Run();
-
-if (top_reader_sp.get() == reader_sp.get()) {
-  if (PopIOHandler(reader_sp))
-break;
-}
-
-while (true) {
-  top_reader_sp = m_input_reader_stack.Top();
-  if (top_reader_sp && top_reader_sp->GetIsDone())
-PopIOHandler(top_reader_sp);
-  else
-break;
-}
-  }
-}
-
 void Debugger::AdoptTopIOHandlerFilesIfInvalid(FileSP , StreamFileSP ,
StreamFileSP ) {
   // Before an IOHandler runs, it must have in/out/err streams. This function
@@ -1005,7 +989,8 @@
 }
 
 void Debugger::PushIOHandler(const IOHandlerSP _sp,
- bool cancel_top_handler) {
+ bool cancel_top_handler,
+ bool asynchronous_if_needed) {
   if (!reader_sp)
 return;
 
@@ -1029,6 +1014,12 @@
 if (cancel_top_handler)
   top_reader_sp->Cancel();
   }
+
+  // Support running the IO handlers synchronously on the current thread if
+  // they aren't running yet.
+  if (asynchronous_if_needed && !m_synchronous_reader_lock.owns_lock()) {
+ExecuteIOHandlers();
+  }
 }
 
 bool Debugger::PopIOHandler(const IOHandlerSP _reader_sp) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -191,13 +191,11 @@
lldb::StreamFileSP );
 
   void PushIOHandler(const lldb::IOHandlerSP _sp,
- bool cancel_top_handler = true);
+ bool cancel_top_handler = true,
+ bool asynchronous_if_needed = false);
 
   bool PopIOHandler(const lldb::IOHandlerSP _sp);
 
-  // Synchronously run an input reader until it is done
-  void RunIOHandler(const lldb::IOHandlerSP _sp);
-
   bool IsTopIOHandler(const lldb::IOHandlerSP _sp);
 
   bool CheckTopIOHandlerTypes(IOHandler::Type top_type,
@@ -403,6 +401,9 @@
   m_script_interpreters;
 
   IOHandlerStack m_input_reader_stack;
+  std::mutex m_synchronous_reader_mutex;
+  std::unique_lock m_synchronous_reader_lock;
+
   llvm::StringMap> m_log_streams;
   std::shared_ptr m_log_callback_stream_sp;
 

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I didn't get a chance to write a test case yet, but you can reproduce the 
problem with the following setup:

  $ echo "script foo = 1" > test.lldb
  $ lldb ./a.out
  (lldb) b main
  (lldb) breakpoint command add -s python
  
frame.GetThread().GetProcess().GetTarget().GetDebugger().HandleCommand("command 
source -s true ./test.lldb")
  DONE
  (lldb) run


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72748/new/

https://reviews.llvm.org/D72748



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

[Lldb-commits] [PATCH] D72748: [lldb/IOHandler] Change the way we manage IO handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham, friss.
Herald added a subscriber: abidh.
Herald added a project: LLDB.
JDevlieghere added a comment.

I didn't get a chance to write a test case yet, but you can reproduce the 
problem with the following setup:

  $ echo "script foo = 1" > test.lldb
  $ lldb ./a.out
  (lldb) b main
  (lldb) breakpoint command add -s python
  
frame.GetThread().GetProcess().GetTarget().GetDebugger().HandleCommand("command 
source -s true ./test.lldb")
  DONE
  (lldb) run


The way the IO handlers are currently managed by the debugger is wrong. The 
implementation lacks proper synchronization between `RunIOHandler` and 
`ExecuteIOHandlers`. The latter is meant to be run by the "main thread", while 
the former is meant to be run synchronously, potentially from a different 
thread. Imagine a scenario where `RunIOHandler` is called from a different 
thread than `ExecuteIOHandlers`. Both functions manipulate the debugger's 
`IOHandlerStack`. Although the `push` and `pop` operations are synchronized, 
the logic to activate, deactivate and run IO handlers is not.

While investigating https://llvm.org/PR44352, I noticed some weird behavior in 
the `Editline` implementation. One of its members (`m_editor_status`) was 
modified from another thread. This happened because the main thread, while 
running `ExecuteIOHandlers` ended up execution the `IOHandlerEditline` created 
by the breakpoint callback thread. Even worse, due to the lack of 
synchronization within the IO handler implementation, both threads ended up 
executing the same IO handler.

Given the way the IO handlers work, I don't see the need to have execute them 
synchronously. When an IO handler is pushed, it will interrupt the current 
handler unless specified otherwise. One exception where being able to run a 
handler synchronously is the sourcing of the `.lldbinit` file in the home and 
current working directory. This takes place *before* `ExecuteIOHandlers` is 
started from `RunCommandInterpreter`, which means that in the new scheme these 
two IO handlers end up at the bottom of the stack. To work around this specific 
problem, I've added an option to run the IO handler synchronously if needed 
(i.e. `ExecuteIOHandlers` is not running yet). When that's the case, any other 
threads are prevented (blocked) from starting to execute the IO handlers. I 
don't think this workaround is necessary for any other handlers.

I've been starting at this for quite a bit and tried a few different 
approaches, but it's totally possible that I missed some. I'm more than open to 
suggestions for more elegant solutions!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72748

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1248,14 +1248,14 @@
 CommandReturnObject ) {
   m_active_io_handler = eIOHandlerBreakpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
-  "", *this, true, _options_vec);
+  "", *this, _options_vec);
 }
 
 void ScriptInterpreterPythonImpl::CollectDataForWatchpointCommandCallback(
 WatchpointOptions *wp_options, CommandReturnObject ) {
   m_active_io_handler = eIOHandlerWatchpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
-  "", *this, true, wp_options);
+  "", *this, wp_options);
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1854,7 +1854,7 @@
   IOHandlerConfirm *confirm =
   new IOHandlerConfirm(m_debugger, message, default_answer);
   IOHandlerSP io_handler_sp(confirm);
-  m_debugger.RunIOHandler(io_handler_sp);
+  m_debugger.PushIOHandler(io_handler_sp);
   return confirm->GetResponse();
 }
 
@@ -2477,7 +2477,11 @@
 
   m_command_source_depth++;
 
-  debugger.RunIOHandler(io_handler_sp);
+  const bool cancel_top_handler = true;
+  const bool asynchronous_if_needed = true;
+  debugger.PushIOHandler(io_handler_sp, cancel_top_handler,
+