[Lldb-commits] [PATCH] D42210: Re-enable logging on the child side of a fork() in lldb-server platform mode

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D42210#979608, @jasonmolenda wrote:

> Jim remembers some problem with logging across a fork()'ed process, Pavel does this ring a bell?  This change might be completely bogus -- but it's very difficult to debug the child side of an lldb-server platform today.


I believe Jim is thinking of https://reviews.llvm.org/D38938. The issue is that if another thread holds the logging mutex while we fork(), the mutex will remain in a bogus state in the child process. This would mean that any operation on the Log subsystem (such as enabling logging) could hang. We hold the mutex for just a couple of instructions (just enough to copy a shared_ptr), but after some code reshuffles, we were hitting this case very often in liblldb.

Now, I don't think this can happen here as at the point where we are forking, the platform process is single-threaded. So, enabling logging here should be safe, but only accidentally. It should be possible to make this always safe, but that will need to be done with a very steady hand. My opinion is we should not bother with that (i.e., just check this in as-is) until we actually need more threads in the platform, particularly as I think the long-term direction here should be to replace the fork with a new thread for handling the connection.

As for testing, the problem we have now is that we have no direct platform tests today -- remote tests will test platform by the virtue of all connections going through it but a standard check-lldb will not even run this code. I have been planning to add tests like these, but I haven't got around to that yet, and I'm not sure when will that happen. If you want to take a stab at it, I can give you some pointers.

BTW: for most debugging needs you should be able to just start the platform without the --server argument, which will disable forking and handle each connection in the main process.


Repository:
  rL LLVM

https://reviews.llvm.org/D42210