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

Tue Jan 21 16:39:58 PST 2020

clayborg added a comment.

In D72748#1830638 <https://reviews.llvm.org/D72748#1830638>, @labath wrote:

> In D72748#1829956 <https://reviews.llvm.org/D72748#1829956>, @JDevlieghere wrote:
>
> > In D72748#1823945 <https://reviews.llvm.org/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 variable
  DONE
  image list

The current IOHandler will be working with STDIN from the debugger, and then we execute "command source /tmp/foo" which will create a new IOHandlerEditline with the file handle for "/tmp/foo" as its input and STDOUT from the debugger as its output and pushed onto the stack. The IOHandler will grab one line, "target stop-hook add" and then push another for the target stop hook to get its commands. This will also use the top IOHandler's file for "/tmp/foo" as input and STDOUT for output, grab its stuff and exit. If the user forgot to add the "DONE" in the list, and the IOHandler just returned and resumed with the old STDIN as input we would be in an intermediate state still waiting for this input, but with the IOHandler stack it would finish up because we would reach EOF on the "/tmp/foo" input.

As each IOHandler is being pushed, it can adopt a different source (STDIN of the debugger object by default, but might be a file when we are using "command source"). So the pushing and popping allow a new IOHandler to be queued up during the CommandObject::DoExecute() function (like a new IOHandler that gets its input from the file from "command source") and as soon as this IOHandler object on the stack is out of input, it can pop itself off the stack. So the stack does provide a nice clean way to do theses kinds of things and the stack allows us to go the the current IOHandler and tell it it is about to not be the top IOHandler and allows it to do setup/teardown etc.

I think there are some complexities for python code where this doesn't work right for things like:

  script
  for i in range(0..1):
    print(i)
  quit()
  image list

But it would be great if it did work with any new solution.

So I just wanted to give some examples of the complex things the stack does for us right now so any modifications can keep this in mind.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72748