[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D100191#2685039 <https://reviews.llvm.org/D100191#2685039>, @labath wrote:

> I like this a lot more than the previous version. The thing I'd like to know is, whether we can replace `m_additional_processes` with something like `m_debugged_processes` (i.e., just have a single list of all processes we are debugging. We could replace all occurrences of `m_debugged_process_up` with `m_current_process`, which just points inside that list. Is there any reason for the "privileged" position of the original process? The way I see it, most of the operations would just work even if we treated them as equals. The only thing which might not work is resuming (or TBE, reporting the stops) of the subprocesses, but if we wanted to be safe, we could prevent that in some other way (preventing the changing of process through Hc; returning "crippled" process instances for subprocess, which return errors on resume operations; ...).
>
> WDYT?

I've tried to limit changes to the minimum but sure, I can try doing that. I suppose it's going to make following children cleaner.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1036
+    std::unique_ptr<NativeProcessProtocol> &child_process) {
+  // Apparently the process has been dealt with by another delegate.
+  if (!child_process)
----------------
labath wrote:
> You no longer have to worry about that...
Don't I? The process plugin puts the new instance in an `std::unique_ptr`, and passes that to all delegates. Only one delegate can take the pointer over. While I don't think we really have a case for multiple delegates doing `NewSubprocess()`, I suppose we should check rather than crash. Or maybe just put an `assert` for it.


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

https://reviews.llvm.org/D100191