1133028 - TSan: data race js/src/gc/Zone.h:167 setGCState

Status: RESOLVED WORKSFORME

(Core :: JavaScript: GC, defect)

Description

[Nathan Froyd [:froydnj]]

Attachment: js-gc-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) [1].

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.

Looks like Zone::gcState_ can be touched by multiple threads at once.

[1] http://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong

Comment 1

[Andrew McCreight [:mccr8]]

Based on the stack, I'm guessing this is a regression from the parallel updating code in compacting GC.
  http://hg.mozilla.org/mozilla-central/rev/63a296e4b6b3

Comment 2

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #1)
Yes that looks like the case.

Nathan, do you have the changeset that was tested?  I'm having trouble matching the line numbers in the stack trace.

Comment 3

[Nathan Froyd [:froydnj]]

I believe this was https://hg.mozilla.org/mozilla-central/rev/38058cb42a0e.  Please let me know if that doesn't make any sense.

Comment 4

[Jon Coppeard (:jonco)]

From the log it seems to be a conflict between reading a zone's gc state in parallel update during compacting and setting it at the start of a subsequent GC.

But that's not possible because we always join on the parallel update tasks before we return to the main program, so I don't know what's going on here.

Comment 5

[Jon Coppeard (:jonco)]

We're not seeing this race in our nightly TSAN runs, so I'm closing this bug.