Helgrind warning about data race in GC (DEBUG-only)

RESOLVED FIXED in Firefox 21

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: billm)

Tracking

unspecified
mozilla22
All
Linux
Points:
---

Firefox Tracking Flags

(firefox21 fixed, firefox22 fixed)

Details

Attachments

(1 attachment)

==26608== Possible data race during read of size 4 at 0x129497BC by thread #5
==26608== Locks held: none
==26608==    at 0x87DA66A: JSRuntime::isHeapBusy() (jscntxt.h:932)
==26608==    by 0x8935688: js::gc::ArenaHeader::checkSynchronizedWithFreeList() const (jsgc.cpp:269)
==26608==    by 0x889D598: js::gc::ArenaHeader::getFirstFreeSpan() const (Heap.h:876)
==26608==    by 0x8950614: bool js::gc::Arena::finalize<JSObject>(js::FreeOp*, js::gc::AllocKind, unsigned long) (jsgc.cpp:313)
==26608==    by 0x894BF9B: bool FinalizeTypedArenas<JSObject>(js::FreeOp*, js::gc::ArenaHeader**, js::gc::ArenaList&, js::gc::AllocKind, js::SliceBudget&) (jsgc.cpp:412)
==26608==    by 0x89359AA: FinalizeArenas(js::FreeOp*, js::gc::ArenaHeader**, js::gc::ArenaList&, js::gc::AllocKind, js::SliceBudget&) (jsgc.cpp:449)
==26608==    by 0x8939D81: js::gc::ArenaLists::backgroundFinalize(js::FreeOp*, js::gc::ArenaHeader*, bool) (jsgc.cpp:1389)
==26608==    by 0x893D833: SweepBackgroundThings(JSRuntime*, bool) (jsgc.cpp:2201)
==26608==    by 0x893EE3D: js::GCHelperThread::doSweep() (jsgc.cpp:2483)
==26608==    by 0x893DED7: js::GCHelperThread::threadLoop() (jsgc.cpp:2327)
==26608==    by 0x893DE3F: js::GCHelperThread::threadMain(void*) (jsgc.cpp:2306)
==26608==    by 0x409754C: _pt_root (ptthread.c:192)
==26608==    by 0x403032F: mythread_wrapper (hg_intercepts.c:219)
==26608==    by 0x4A2CE99: start_thread (pthread_create.c:308)
==26608==    by 0xB211CBC: clone (clone.S:112)
==26608==
==26608== This conflicts with a previous write of size 4 by thread #1
==26608== Locks held: none
==26608==    at 0x8948017: js::gc::AutoTraceSession::~AutoTraceSession() (jsgc.cpp:3956)
==26608==    by 0x894814A: AutoGCSession::~AutoGCSession() (jsgc.cpp:3968)
==26608==    by 0x894958C: GCCycle(JSRuntime*, bool, long, js::JSGCInvocationKind, JS::gcreason::Reason) (jsgc.cpp:4363)
==26608==    by 0x8949B27: Collect(JSRuntime*, bool, long, js::JSGCInvocationKind, JS::gcreason::Reason) (jsgc.cpp:4490)
==26608==    by 0x8949CF4: js::GC(JSRuntime*, js::JSGCInvocationKind, JS::gcreason::Reason) (jsgc.cpp:4513)
==26608==    by 0x8813DE9: JS_GC(JSRuntime*) (jsapi.cpp:2849)
==26608==    by 0x40D587: main (xpcshell.cpp:1950)

Basically we're reading and writing JSRuntime::heapState without holding any locks. This looks to be DEBUG-only so not sure what to do.
Created attachment 721944 [details] [diff] [review]
fix

This is one actually pretty easy to fix.
Assignee: general → wmccloskey
Status: NEW → ASSIGNED
Attachment #721944 - Flags: review?(terrence)
Comment on attachment 721944 [details] [diff] [review]
fix

Review of attachment 721944 [details] [diff] [review]:
-----------------------------------------------------------------

Very nice.
Attachment #721944 - Flags: review?(terrence) → review+
Bill, you're my hero! Thanks!
Something in this push caused bustage. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2da17db2a304

https://tbpl.mozilla.org/php/getParsedLog.php?id=20443040&tree=Mozilla-Inbound

In file included from ../../../js/src/jsalloc.h:11:0,
                 from ../../../js/src/jsalloc.cpp:8:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsalloc.o] Error 1
make[5]: *** Waiting for unfinished jobs....
In file included from ../../../js/src/jsalloc.h:11:0,
                 from ../../../js/src/jsapi.h:24,
                 from ../../../js/src/jscntxt.h:19,
                 from ../../../js/src/jscompartment.h:15,
                 from ../../../js/src/jsanalyze.h:15,
                 from ../../../js/src/jsanalyze.cpp:9:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsanalyze.o] Error 1
In file included from ../../../js/src/jsarray.cpp:18:0:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsarray.o] Error 1
In file included from ../../../js/src/jsapi.cpp:22:0:
../../../js/src/jsutil.h: In function 'void js::PodArrayZero(T (&)[N]) [with T = volatile unsigned int, unsigned int N = 4032u]':
../../../js/src/gc/Heap.h:707:28:   instantiated from here
../../../js/src/jsutil.h:214:5: error: invalid conversion from 'volatile void*' to 'void*'
../../../js/src/jsutil.h:214:5: error:   initializing argument 1 of 'void* memset(void*, int, size_t)'
make[5]: *** [jsapi.o] Error 1
https://hg.mozilla.org/mozilla-central/rev/1c0d0c9c6ee7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 721944 [details] [diff] [review]
fix

This is a DEBUG-only issue. However, it looks like we might be hitting it on tinderbox in bug 848587. It might be nice to backport this to eliminate the intermittent orange on aurora. It doesn't seem to happen on other branches.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Not sure, possibly bug 601075.
User impact if declined: none
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): None. Only affects debug builds.
String or UUID changes made by this patch: None.
Attachment #721944 - Flags: approval-mozilla-aurora?

Comment 9

5 years ago
(In reply to Bill McCloskey (:billm) from comment #8)
> However, it looks like we might be hitting it on
> tinderbox in bug 848587.

You mean bug 847717. :)
Yes. Hopefully this potential cycle in bugzilla won't destroy the universe.
Comment on attachment 721944 [details] [diff] [review]
fix

Low risk patch helps avoid intermittent orange, approving for uplift.
Attachment #721944 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox21: --- → fixed
status-firefox22: --- → fixed
You need to log in before you can comment on or make changes to this bug.