[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

This change is fine for what it does, but I don't think the model that it allows is really supportable.  If you have multiple listeners on the process events, and Listener A wants to do a "step" when notified that the process has stopped, but Listener B wants to do a "Continue", then there's no way to coordinate these and we're just allowing race conditions in controlling the process.  I think you really need to have one agent that controls the process, and then other threads that are passive listeners.

My intention for this was to have the primary Listener (the one that was registered with the process when it was created) be the "controlling listener".  We would ensure that that listener gets notified first, and add some kind of handshake where the controlling listener relinquishes control, after which the other listeners will be informed.

Anyway, my feeling is we should think a bit more about policy w.r.t. fetching process events from multiple Process listeners before we allow them.

There should also be some test cases for handling a process with multiple listeners, since that's not something we've supported before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86652