ignore setLoggerLevel: when compiling w/thread-sanitizer on

[johnkdoe]

during unit-testing with the thread-sanitizer on, setLoggerLevel: was flagged unnecessarily as a potential data-race.

__sharedLoggerLevel is already marked volatile, and the location of the data race indicated was a debug logging message .

[coveralls]

Coverage remained the same at 76.243% when pulling d6c03e1fd50c7cd73c2ca1012494ae5f84f7fa8a on johnkdoe:develop-with-thread-sanitizer into 065fb0feaf0094bc9bf2eaf7afe8d2f4279be3fe on twitter:develop.

[NSProgrammer]

What unit tests? How can I repro? The move to stdatomic SHOULD have solved this and for thread sanitizer to still elicit warnings makes me want to double check that we are properly accessing the variable everywhere.

[kgoodier]

I did some research and volatile is not necessary with the std::atomic variables, of which __sharedLoggerLevel is one. We should probably skip this change, and just remove the volatile keyword. I'm happy to do that, but I don't know how to repro this sanitizer error you're seeing; @johnkdoe can you try that change and see if stops?

UPDATE: std::atomic is for C++, which is not what we are using here. We are using <stdatomic.h>, the C version, which uses the _Atomic keyword. It is much more unclear about the use of the volatile keyword, but seems to be independent of it. They can be used together or independently. I still say we don't need the 'volatile' keyword, but it's not hurting anything.

[johnkdoe]

for Twitter, was getting this just running the "Bluebird Enterprise" scheme, performing unit tests with thread sanitizer turned on. can try to re-test with __sharedLoggerLevel established as a non-volatile …

[johnkdoe]

deleted last 2 comments with stack-traces of thread-sanitizer … and also got the latest proper SHA1 for the develop branch …

[kgoodier]

I guess the thread sanitizer is technically correct, __sharedLoggerLevel is, by design, a race between writes & reads. It doesn't matter though, as long as it is updated atomically. I'm surprised the thread sanitizer isn't more cognizant or accepting of usage patterns like this. I suppose the only thing we can do is disable it the check, as you've proposed in this request.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Kirk Beitz seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}