Closed Bug 1261083 Opened 4 years ago Closed 4 years ago

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


(Core :: JavaScript: GC, defect)

Not set



Tracking Status
firefox48 --- affected
firefox50 --- fixed


(Reporter: jseward, Assigned: jonco)


(Whiteboard: [tsan])


(2 files)

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

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)

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.

[2] _How to miscompile programs with "benign" data races_:
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.
Flags: needinfo?(jseward)
The changes in bug 1266107 should also reduce the number of TSAN warnings related to compacting GC.
(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.
Flags: needinfo?(jseward)
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.
Assignee: nobody → jcoppeard
Attachment #8764645 - Flags: review?(terrence)
Attachment #8764645 - Flags: review?(terrence) → review+
Pushed by
Update base shapes in a separate phase at the start of compacting GC r=terrence
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.