DavidSpickett wrote:

If I understand correctly, this is being tested on Linux so we have pthreads 
created as joinable by default:
> The default setting of the detach state attribute in a newly
>  initialized thread attributes object is PTHREAD_CREATE_JOINABLE.

This means that even though the monitoring thread has done its work, it sits 
around waiting for someone to join it. Which lldb-server does not.

So we could either:
* join the thread (but when would we do that, what if the monitoring thread 
gets stuck waiting for some event?)
* create the thread as detached (which is what you've done)

Maybe there is a place to safely join but
> The proposed solution at this stage is to simply detach internal threads, so 
> that the OS can release memory associated with these threads. That is to say, 
> it seems to me that lldb-server doesn't have appropriate mechanisms to track 
> spawned threads, therefore a more thoughtful approach would likely require 
> changes to a substantial part of the existing logic.

I think you are making the point here that anything along the lines of joining 
threads later would be a lot more work to implement.

Your change would not be merged as is because the POSIX-specific detail needs 
to be hidden in lower level classes (surprised that it built on Windows, 
assuming CI tried to), but the idea would be the same. I was thinking of using 
https://man7.org/linux/man-pages/man3/pthread_attr_setdetachstate.3.html in 
`ThreadLauncher::LaunchThread`.

A hack to do that for all threads created crashes a whole bunch of tests, which 
should have been obvious to me. But it showed me that at some callers are 
trying to join the threads later. So we'll need to pass down a flag, like we've 
done with `min_stack_byte_size`.

ThreadLauncher.cpp is also in "common" but at least it already has ifdefs for 
Windows vs. POSIX, so my first guess is this is the place to do this.

https://github.com/llvm/llvm-project/pull/177572
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to