[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

Thu Jul 9 09:47:27 PDT 2020

JDevlieghere added a comment.

In D83446#2141713 <https://reviews.llvm.org/D83446#2141713>, @labath wrote:

> Well, this is an interesting problem... IIUC, what's happening is that the printing of the "stop reason" in the "event handler thread" is generating some packets (to fetch the stop reason), and then the "cont" command produces some (c) too...

Yup

> I'm wondering if this problem does not go beyond reproducers... The fact that we can have two threads running in parallel, doing stuff, without much synchronization seems like a bad idea in general. In theory, something similar could happen during normal operation as well, if the user was fast enough to type "cont<Return>" before the stop reason is printed. In the non-reproducer mode we wouldn't crash with the unexpected packet assertion, but we would probably do something unexpected nonetheless. Of course, a real human user won't be fast enough to do that, but I wouldn't rule out this being the cause of the flakiness of some of our pexpect tests. And I'm not sure what happens when sourcing command files, which is fairly similar to running a reproducer. How would the events be processed there?

Agreed. I believe at some point (long before I started working on LLDB) all the GDB communication was limited to a single thread, but the overhead of having to talk and synchronize with that thread was too costly and it got relaxed.

> So, I'm wondering if we shouldn't add some form of synchronization here, which would be independent of the reproducer mode we're running in. That would solve the reproducer problem as a consequence, but it seems like it would be generally useful. One way to do that could be to send some sort of a ping event (EventDataReceipt ?) to the event handler thread before or after running a command, and wait for it to report back. That should ensure that all events produced by the previous command have been fully processed...

How would this be different from synchronous mode and how would you make this work in asynchronous mode?

As an aside: the first thing I considered here was making this particular command //synchronous// by blocking until we get the stop event. During replay the execution is always synchronous (which is something I don't like and was hoping to get rid off with this patch). That would only solve the problem for this command and adds a bunch of assumptions about connections resulting in stops. I know this is always true for gdb platforms, but maybe not for other platforms. Also currently the debugger is the only one that deals with these events, I'm not super excited to have that logic spread out with some commands handling it synchronously and others doing it asynchronously.

Based on your description I don't yet see how the `EventDataReceipt` thingy would work. If we block from the moment the event is posted, then what's the goal of having the event handling logic running in a separate thread? If we block between when the event is being handled until it finished, then how would you avoid a race between when the event is broadcast and when it's being handled? Can you elaborate a bit on this?

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D83446