Created attachment 8547773 [details] tsan-cache-race.txt [cribbing from decoder's script, hopefully he won't mind] The attached logfile shows a thread/data race detected by TSan (ThreadSanitizer). Typically, races reported by TSan are not false positives, but it is possible that the race is benign. Even in this case though, we should try to come up with a fix unless this would cause unacceptable performance issues. Also note that seemingly benign races can possibly be harmful (also depending on the compiler and the architecture) . If the bug cannot be fixed, then this bug should be used to either make a compile-time annotation for blacklisting or add an entry to the runtime blacklist.  http://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong
This needs a little bit more investigation. More then just a race on an atomic happens here. I smell a bug, but fix for that will not fix the warning (tho, that may become truly benign with the fix).
Assignee: nobody → honzab.moz
|enum class nsresult : uint32_t| declared class member is not atomically accessible on 64 bit builds. mozilla::Atomic with just rel_acq ordering will do here.
Status: NEW → ASSIGNED
Not sure why this ni was added...
Created attachment 8561432 [details] [diff] [review] v1 Nathan, feel free to forward to e.g. :michal novotny.
Comment on attachment 8561432 [details] [diff] [review] v1 Review of attachment 8561432 [details] [diff] [review]: ----------------------------------------------------------------- ReleaseAcquire is sort of odd to use in this context, but I think it makes sense, because you just want to ensure that you see all prior writes to the location. I think making it Relaxed would work too; that's effectively what you have now, but the compiler would then know that you're explicitly specifying that the races (such as they are) are OK. I'm going to forward the review to Michal because I don't know enough about the cache to say whether the hit you take from ReleaseAcquire vs. Relaxed (on ARM devices, anyway) is OK here.
Attachment #8561432 - Flags: review?(michal.novotny) → review+
since this is the same try run as for bug 881830 - are this failures in the try run related to this patch ?
Created attachment 8564176 [details] [diff] [review] v1 (ci msg) https://treeherder.mozilla.org/#/jobs?repo=try&revision=c34ac09f8042
Attachment #8561432 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.