[Lldb-commits] [PATCH] use std::atomic<> to protect variables being accessed by multiple threads
tfiala at google.com
Mon Sep 15 07:55:56 PDT 2014
Just for clarification, is this a LGTM? Or are you wanting Shawn to spend
more time looking at the bigger picture behind some of the locking?
On Fri, Sep 12, 2014 at 12:37 PM, <jingham at apple.com> wrote:
> That looks okay. Thanks for looking at this. Seems to me that if you're
> only synchronization is locking access to some flag, then you're either
> doing something fairly weak (like the log example) or missing some real
> synchronization... So it's worth viewing the necessity of the change with
> > On Sep 12, 2014, at 11:15 AM, Shawn Best <sbest at blueshiftinc.com> wrote:
> > Hi Jim, thanks for the feedback.
> > I agree these may fall into the category of 'formally correct, but not
> strictly necessary.' One good thing is it makes clear the intention these
> variables are being directly accessed from multiple threads.
> > Something I noticed about thread sanitizer is that it will complain if 2
> threads access the same memory without a synchronization mechanism between
> them, even if those accesses are separated by a good amount of time (or
> some other kind of logic).
> > Looking at closer at exit status, the problem, I think I should add a
> mutex to Process::SetExitStatus() , GetExitStatus(), GetExitDescription().
> This should prevent an exit code and exit string potentially not matching.
> > Yes, ProcessPosix, ProcessFreeBSD directly setting the variable, then
> calling a function to do so, looks a little weird, I've cleaned that up.
> > As far as I can tell, if someone were to turn logging on/off in the
> middle of a function dumping log information... the worst that would happen
> is it would not start/stop a bit late. The typical usage pattern will
> check the flag once at the beginning of the function and keep the pointer
> for the duration of the function. I don't think that would be anything a
> user would notice.
> > Shawn.
> > http://reviews.llvm.org/D5302
> > Files:
> > include/lldb/Core/Communication.h
> > include/lldb/Core/ConnectionFileDescriptor.h
> > include/lldb/Target/Process.h
> > source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
> > source/Plugins/Process/POSIX/ProcessPOSIX.cpp
> > source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
> > source/Target/Process.cpp
> > source/lldb-log.cpp
> > <D5302.13649.patch>_______________________________________________
> > lldb-commits mailing list
> > lldb-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the lldb-commits