[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

[Pavel Labath via lldb-commits]

On 15 October 2017 at 23:59, Zachary Turner <zturner at google.com> wrote:
> what are we using fork for, out of curiosity?  It seems pretty hard to make
> this work.  Logging isn’t the only thing in the program that might be
> holding a mutex, so it seems like this could still happen.
>
> Seems like mixing fork and mutexes is just a fundamentally bad idea, unless
> I’m missing something (which i very well could be)

We are using fork() (+exec()) to start a new process, which is kinda
hard to avoid, since we're in the business of running processes. :)

In general, mixing fork and anything is a bad idea and code that runs
between fork() and exec() needs to be written with a very steady hand.
(there usually isn't that much of it -- our function is 100 lines
long). Generally, one tries to use purely c library functions here,
but we also use some low level lldb functions, mainly for decoding
ProcessLaunchInfo struct. Some of them probably do some logging (I
haven't looked at which ones, but this log disabling code has been
there for ages).

Normally, it is expected that the author of this code will make sure
that all code he uses is safe to call in this funky state. However,
logging is a bit special in that in that logging calls can crop up
anywhere and we expect them to be safe to use from any context. So
even if we made sure these functions do no logging that, that can
easily change in the future.

PS: One way we could encapsulate this better would be to use the
pthread_atfork() call to register callback. That way, all of this
nastiness could be hidden in the Log class. (However, I don't think
this is particularly bad either, as ProcessLauncher is one of the
lowest layers anyway)

>
> On Sun, Oct 15, 2017 at 3:15 PM Pavel Labath <labath at google.com> wrote:
>>
>> On 15 October 2017 at 23:04, Zachary Turner <zturner at google.com> wrote:
>> > Doesn’t DisableAllLogChannels acquire a scoped lock? If so wouldn’t it
>> > just
>> > release at the end of the function?
>>
>>
>> The thing is, it is unable to acquire the lock in the first place,
>> because the mutex is already "locked". So, the sequence of events is
>> process 1, thread 1: acquire lock
>> process 1, thread 2: fork(), process 2 is created
>> process 1 thread 1: release lock
>> everything goes well from now on in process 1...
>> process 2, thread one (and only). acquire lock:
>> BANG.
>> Process 2 is deadlocked because there is noone to release the lock there.

On 15 October 2017 at 23:59, Zachary Turner <zturner at google.com> wrote:
> what are we using fork for, out of curiosity?  It seems pretty hard to make
> this work.  Logging isn’t the only thing in the program that might be
> holding a mutex, so it seems like this could still happen.
>
> Seems like mixing fork and mutexes is just a fundamentally bad idea, unless
> I’m missing something (which i very well could be)
>
> On Sun, Oct 15, 2017 at 3:15 PM Pavel Labath <labath at google.com> wrote:
>>
>> On 15 October 2017 at 23:04, Zachary Turner <zturner at google.com> wrote:
>> > Doesn’t DisableAllLogChannels acquire a scoped lock? If so wouldn’t it
>> > just
>> > release at the end of the function?
>>
>>
>> The thing is, it is unable to acquire the lock in the first place,
>> because the mutex is already "locked". So, the sequence of events is
>> process 1, thread 1: acquire lock
>> process 1, thread 2: fork(), process 2 is created
>> process 1 thread 1: release lock
>> everything goes well from now on in process 1...
>> process 2, thread one (and only). acquire lock:
>> BANG.
>> Process 2 is deadlocked because there is noone to release the lock there.