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

This revision was automatically updated to reflect the committed changes. Closed by commit rL316368: Logging: Disable logging after fork() (authored by labath). Changed prior to commit: https://reviews.llvm.org/D38938?vs=119710=119920#toc Repository: rL LLVM https://reviews.llvm.org/D38938

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

eugene accepted this revision. eugene added inline comments. This revision is now accepted and ready to land. Comment at: source/Utility/Log.cpp:333 + +void Log::DisableLoggingChild() { + for (auto : *g_channel_map) A little comment here describing nature of a

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

labath updated this revision to Diff 119710. labath added a comment. The previous fix had a problem where a more paraniod libc (e.g., android, but I suspect freebds would do that as well) would detect that the thread unlocking the mutex is not the same one as the one that locked, and refused to

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL316173: Logging: Make sure logging machinery is in a consistent state after forking (authored by labath). Repository: rL LLVM https://reviews.llvm.org/D38938 Files:

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Much better. https://reviews.llvm.org/D38938 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

labath updated this revision to Diff 119315. labath added a comment. Solve the problem using pthread_atfork(). This way the locks are properly taken before forking and released in both child and parent processes. This behavior is encapsulated within the Log class and is completely transparent to

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

See my other message. In a ptrace based system the inferior has to call PT_TRACEME to signal it should be stopped at the first instruction. So you do need to run that code. As I said, Apple added an extension to posix_spawnp to do this for us. Jim > On Oct 16, 2017, at 2:17 PM, Zachary

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

This thread behavior over fork is the same on Mach, BTW. Apple extended posix_spawnp to take a “Stop at first instruction” attribute, basically just running PT_TRACEME for us. That has ended up being very handy because we get all the nice cleanup behavior that pthread_spawn does, but still

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

Ahh wait, sorry. I meant posix_spawn, not execve On Mon, Oct 16, 2017 at 2:16 PM Zachary Turner wrote: > I guess what I mean is, my understanding is that the only "advantage" (if > you can even call it that) of using fork + exec over execve is that fork + > exec allows you

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

I guess what I mean is, my understanding is that the only "advantage" (if you can even call it that) of using fork + exec over execve is that fork + exec allows you to run additional code between the fork and the exec. Do we actually rely on that? Why do we need it to do fork at all? Why can't

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

On linux when you call fork the new process will only have the thread what called fork. Other threads will be ignored with leaving whatever dirty state they had left in the new process. Regarding execve it doesn't do fork so we would have to do fork & execve what have the same issue (actually we

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

On Sun, Oct 15, 2017 at 3:15 PM Pavel Labath wrote: > On 15 October 2017 at 23:04, Zachary Turner 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

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

eugene added a comment. DisableUnlocked methods makes me uneasy. Maybe we can take a log lock before forking and then properly release it in both parent and child processes? https://reviews.llvm.org/D38938 ___ lldb-commits mailing list

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

clayborg added a comment. Never mind, re-reading your original comment made it clear. https://reviews.llvm.org/D38938 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

clayborg added a comment. Who is holding the lock and then fork'ing? Seems like we should fix that? I thought all log calls should be "lock, log, unlock". Why is this causing problems? https://reviews.llvm.org/D38938 ___ lldb-commits mailing list

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

On 15 October 2017 at 23:59, Zachary Turner 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

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

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

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

On 15 October 2017 at 23:04, Zachary Turner 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".

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

Doesn’t DisableAllLogChannels acquire a scoped lock? If so wouldn’t it just release at the end of the function? On Sun, Oct 15, 2017 at 2:42 PM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath created this revision. > > We had a bug where if we had forked (in the

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

labath created this revision. We had a bug where if we had forked (in the ProcessLauncherPosixFork) while another thread was writing a log message, we would deadlock. This happened because the fork child inherited the locked log rwmutex, which would never get unlocked. This meant the child got