jingham added a comment. In D83446#2143464 <https://reviews.llvm.org/D83446#2143464>, @labath wrote:
> In D83446#2142564 <https://reviews.llvm.org/D83446#2142564>, @JDevlieghere > wrote: > > > Seems like my thoughts got lost a bit with all the inline replies: we can > > solve this particular issue by making `process connect` block in > > synchronous mode. The fact that that's not happening today is a bug beyond > > the reproducers. I don't think we should change the current behavior in > > asynchronous mode. I think it's probably worthwhile to do that even if we > > decide to add extra synchronization for the reproducers in which case this > > would no longer be an example of a problem solved by the current patch. > > > I'm starting to understand why you say "process connect" is not > blocking/synchronous. Although it is waiting for the "stopped" event to be > sent to the public queue (here > <https://github.com/llvm/llvm-project/blob/master/lldb/source/Target/Process.cpp#L3119>), > it is not waiting for it to be processed by the public queue. The "process > continue" command OTOH, is kind of waiting for the public queue processing to > finish. I say only "kind of" because it does not actually process it on the > _real_ public queue -- instead it "hijacks" the events and processes then on > its own little event loop > <https://github.com/llvm/llvm-project/blob/master/lldb/source/Target/Process.cpp#L776>. > The part that I got wrong was that I was assuming that the hijacked event > loop would rebroadcast the stop event to the real public queue. In fact, it > doesn't, which means that the stop event processing is completely finished by > the time that the "process continue" command returns. > > It seems reasonable that "process connect" (and it's SB equivalent) should > behave the same way. This would also explain why so many of the "gdb-client" > tests need to use `lldbutil.expect_state_changes` when connecting to the mock > server, even though they are running in synchronous mode (the only other > tests using this function explicitly choose to use the async mode). > > For async mode, we can say that the user needs to ensure that the event is > handled before he can issue other commands -- this is the same situation as > with "process continue". Of course, in the asynch case, the reproducers are > "the user" and they will need to do something about this. But maybe we don't > need to cross that bridge right now? The "synchronous" mode contract is that by the time the command completes, there should be no more event work to be done. That's why the event should be consumed and not rebroadcast. It seems to me "process connect" is just breaking the synch mode contract. I think reproducers in truly async mode (particularly where you can have multiple processes each with different listeners) is going to be challenging for reproducers. But sync mode shouldn't be that hard, provided it is implemented correctly everywhere. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83446/new/ https://reviews.llvm.org/D83446 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
