[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1050
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([&, pid](MainLoopBase &loop) {
+    m_debugged_processes.erase(pid);
----------------
The universal reference capture is dangerous for callbacks that outlive the enclosing function. I think that `[this,pid]` would suffice.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1196
+  if (!bool(m_extensions_supported &
+            NativeProcessProtocol::Extension::multiprocess))
+    assert(!m_stdio_handle_up);
----------------
mgorny wrote:
> labath wrote:
> > I don't think this is right. I believe the important distinction is whether we have non-stop mode enabled or not, because in all-stop mode we are not able to send stdio while stopped, regardless of how many processes we're debugging.
> Well, the code exploded in all-stop mode as well because in order to kill multiple processes, we effectively resume them all. I suppose there could be a way around it (synchronously killing one process after another and waiting for everything to happen) but I'm not convinced this is really worth the added complexity.
I don't think this needs to be complex at all. What we need to basically do, is call StartSTDIOForwarding whenever the protocol allows us to send stdio, and StopSTDIOForwarding when we cannot. When we supported just a single process, the easiest way to achieve this was to hook into the started/stopped events of that process. Now that's no longer true, so maybe we just need to hook it up elsewhere.

I think starting could be done directly from the packet handlers (c/s/vCont/...). And those handlers already have to check for nonstop mode, so any deviations could be handled there:
```
if (all_stop) {
  StartSTDIOForwarding(); // Can forward from now until the stop-reply packet is send
  return Success;
} else {
  SendOKResponse();
  // Can we forward now? Or maybe it can be sent all the time?
}
```

Stopping could probably stay coupled with the sending of the stop-reply packet (i.e., handling of the process event), since it's the stop reply which determines whether the stdio packet can be sent.


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

https://reviews.llvm.org/D127500