1261083 - TSan: data race js/src/jsgc.cpp in js::gc::MovingTracer::onShapeEdge(Shape**)

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Julian Seward [:jseward]]

The attached logfile shows a thread/data race detected by TSan (ThreadSanitizer).

* Specific information about this bug

This may be fallout from bug 1257093.  It's mostly unclear to me, but
at first glance it looks as if there is a race on
RelocationOverlay::magic_.

Note that there are 3 different races reported multiple times:

- isForwarded reading, followed later by onShapeEdge writing

- onShapeEdge writing, followed later by isForwarded reading

- isForwarded reading, followed later by
                       DoCallback<jsid>(JS::CallbackTracer* ..) writing

The attached logfile shows only one race for the first two, since the
stacks are the same, just that the events happen in a different order.

* General information about TSan, data races, etc.

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][2].

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.

[1] http://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong
[2] _How to miscompile programs with "benign" data races_: https://www.usenix.org/legacy/events/hotpar11/tech/final_files/Boehm.pdf

Comment 1

[Julian Seward [:jseward]]

Attachment: TSan complaints (race-onShapeEdge)

Comment 2

[Jon Coppeard (:jonco)]

Julian, does this still happen since bug 1260371 landed?  It's not a fix but it will probably change the TSAN signature.  I would expect a lower incidence of a slight different signature since that change.

Comment 3

[Jon Coppeard (:jonco)]

The changes in bug 1266107 should also reduce the number of TSAN warnings related to compacting GC.

Comment 4

[Julian Seward [:jseward]]

(In reply to Jon Coppeard (:jonco) from comment #2)
> Julian, does this still happen since bug 1260371 landed?

Yes .. this does still happen, even with the fixes 
from both 1260371 and 1266107 in place.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: update-baseshapes-first

We need to update all base shapes in a separate phase before any shapes get updated.  Since there are relatively few base shapes, this should not be a problem.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5412891fa76
Update base shapes in a separate phase at the start of compacting GC r=terrence

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e5412891fa76