[Lldb-commits] [PATCH] use std::atomic<> to protect variables being accessed by multiple threads

[Todd Fiala]

All good - I just wanted to double check since I jumped the gun on Jason
last time I took it the wrong way :-)

On Mon, Sep 15, 2014 at 11:06 AM, Jim Ingham <jingham at apple.com> wrote:

> Sorry if I wasn’t clear, this looks fine.  Check it in.
>
> Jim
>
> On Sep 15, 2014, at 7:55 AM, Todd Fiala <tfiala at google.com> wrote:
>
> Hey Jim,
>
> 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?
>
> -Todd
>
> 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
>> skepticism.
>>
>> Jim
>>
>> > 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
>
>
>


-- 
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140915/68257859/attachment.html>