================
@@ -150,12 +153,17 @@ static void 
client_handle(GDBRemoteCommunicationServerPlatform &platform,
   printf("Disconnected.\n");
 }
 
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
 static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
-  std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex);
-  gdbserver_portmap.FreePortForProcess(pid);
+  if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+    // If not running as a server and the platform connection is closed
+    if (!g_terminate) {
+      g_terminate = true;
----------------
labath wrote:

I think I know the problems with detached threads pretty well (remember how I 
said I found bugs in lldb's dependencies -- [one of 
them](https://sourceware.org/bugzilla/show_bug.cgi?id=19951) is in glibc's 
pthread_detach). And I'm not interested in changing the design of the 
monitoring callback. It's detached and it's going to stay detached for a long 
time to come.

The thing I'm questioning is whether the code you are running inside the 
callback has to be detached (because the fact that the outer callback machinery 
is detached doesn't mean that the callback itself has to be detached).

The problems with detached threads are a runtime property. If a thread 
*accesses* something after it has been destroyed, you have a bug. I emphasised 
"access" because the access has to happen. Mere existence of a 
pointer/reference that would permit that access is not in itself a problem. So 
code like
```
int main() {
  std::condition_variable cv;
  std::thread([&]{cv.notify_one();}).detach();
  cv.wait();
  return;
}
```
is safe because even though the detached thread holds a reference to the local 
object (and may continue to hold it after its destroyed), the program ensures 
that the (only) usage of that reference happens while the object is alive. If I 
removed `cv.wait()` then we'd have a problem.

Why I'm mentioning this? Because this is exactly the pattern we have (or one 
that I think we should have) here, just instead of `cv.wait()` we have 
`mainloop.Run()` and `cv.notify()` is replaced by 
`mainloop.RequestTermination()`

If we can make sure things work this way, we're safe, or at least the (0.1%) 
problems are "not our problem". If we can't undetach this code, then we're 
exacerbating the problem, because even if someone wanted to make the undetach 
the monitoring callback machinery, he could make that work because the code 
inside it would be detached.

https://github.com/llvm/llvm-project/pull/104238
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to