1805457 - ThreadSanitizer: data race /builds/worker/checkouts/gecko/js/src/gc/Compacting.cpp:445:14 in onEdge<JSObject>

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[Treeherder Bug Filer]

Filed by: jcoppeard [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=399099761&repo=try
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/ZUuY22JWTci9hu_prlPsfA/runs/0/artifacts/public/logs/live_backing.log

---

As reported by RyanVM when running mochitest-devtools tests under TSAN.

https://treeherder.mozilla.org/logviewer?job_id=399099761&repo=try&lineNumber=4463-4693

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1805457 - Don't trace Wasm instance's object_ edge more than once r?sfink

Previously this traced wasm::Instance's object_ edge when it needed to trace
the WasmInstanceObject. That was kind of wrong but it worked except for the
fact that it meant that this edge could be traced from different threads at the
same time during compacting (and probably in parallel marking too).

It's clearer to get the object and trace that if that is what is required.

I removed the comment from Instance::tracePrivate because it makes it sound
like you can get away without tracing every edge if there's some justification.
This is potentially confusing. All edges must be traced.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bea0e8a2bc0
Don't trace Wasm instance's object_ edge more than once r=sfink

Comment 3

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3bea0e8a2bc0

Comment 4

[Jon Coppeard (:jonco)]

This failure is still showing up when running TSAN tests on try.

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9838c03ad213
Backed out changeset 3bea0e8a2bc0 as requested by jonco. CLOSED TREE

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1805457 - Add TraceInstanceEdge to trace edges from GC things to wasm::Instances r?sfink

The previous attempt was still a data race because it read the object pointer
out of the Instance during compacting when it could be being updated on a
separate thread.

This skips tracing these edges in compacting and makes it clear that this is
unnecessary for edges to non-GC-thing Instance objects.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0a3f49a57f4
Add TraceInstanceEdge to trace edges from GC things to wasm::Instances r=sfink

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/9838c03ad213

Comment 9

[Norisz Fay [:noriszfay]]

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