Closed Bug 850130 Opened 11 years ago Closed 4 months ago

Helgrind warning about data race on mark bits

Categories

(Core :: JavaScript Engine, defect)

All
Linux
defect

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: bent.mozilla, Unassigned)

Details

==2865== Possible data race during write of size 8 at 0x148FCB70 by thread #1
==2865== Locks held: none
==2865==    at 0x8F1E7FC: UnmarkGrayGCThing(void*) (Heap.h:703)
==2865==    by 0x8F1EAC8: UnmarkGrayChildren(JSTracer*, void**, JSGCTraceKind) (Marking.cpp:1577)
==2865==    by 0x8F243F7: void MarkInternal<js::types::TypeObject>(JSTracer*, js::types::TypeObject**) (Marking.cpp:176)
==2865==    by 0x8F237D3: void js::gc::Mark<js::types::TypeObject>(JSTracer*, js::EncapsulatedPtr<js::types::TypeObject, unsigned long>*, char const*) (Marking.cpp:205)
==2865==    by 0x8F112D7: js::gc::MarkTypeObject(JSTracer*, js::EncapsulatedPtr<js::types::TypeObject, unsigned long>*, char const*) (Marking.cpp:342)
==2865==    by 0x8E01C58: js::ObjectImpl::markChildren(JSTracer*) (ObjectImpl.cpp:304)
==2865==    by 0x8F1ACF4: js::gc::MarkChildren(JSTracer*, JSObject*) (Marking.cpp:945)
==2865==    by 0x8F1E383: js::TraceChildren(JSTracer*, void*, JSGCTraceKind) (Marking.cpp:1456)
==2865==    by 0x89BF1FD: JS_TraceChildren(JSTracer*, void*, JSGCTraceKind) (jsapi.cpp:2454)
==2865==    by 0x8F1F0A2: JS::UnmarkGrayGCThingRecursively(void*, JSGCTraceKind) (Marking.cpp:1621)
==2865==    by 0x65AB5D7: xpc_UnmarkGrayContext(JSContext*) (GCAPI.h:236)
==2865==    by 0x6C9030D: nsXPConnect::Pop(JSContext**) (nsXPConnect.cpp:2132)
==2865==    by 0x67A8051: mozilla::dom::workers::WorkerRunnable::Run() (WorkerPrivate.cpp:1625)
==2865==    by 0x7D328B5: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:637)
==2865==    by 0x7CB86C8: NS_ProcessPendingEvents_P(nsIThread*, unsigned int) (nsThreadUtils.cpp:188)
==2865==    by 0x7CC4B62: mozilla::ShutdownXPCOM(nsIServiceManager*) (nsXPComInit.cpp:557)
==2865==    by 0x7CC4926: NS_ShutdownXPCOM_P (nsXPComInit.cpp:513)
==2865==    by 0x4056840: NS_ShutdownXPCOM (nsXPComStub.cpp:134)
==2865==    by 0x40D7B2: main (xpcshell.cpp:1972)
==2865== 
==2865== This conflicts with a previous read of size 8 by thread #5
==2865== Locks held: none
==2865==    at 0x8AFADAD: bool js::gc::Arena::finalize<js::types::TypeObject>(js::FreeOp*, js::gc::AllocKind, unsigned long) (Heap.h:678)
==2865==    by 0x8AF1F5D: bool FinalizeTypedArenas<js::types::TypeObject>(js::FreeOp*, js::gc::ArenaHeader**, js::gc::ArenaList&, js::gc::AllocKind, js::SliceBudget&) (jsgc.cpp:418)
==2865==    by 0x8ADB3E6: FinalizeArenas(js::FreeOp*, js::gc::ArenaHeader**, js::gc::ArenaList&, js::gc::AllocKind, js::SliceBudget&) (jsgc.cpp:463)
==2865==    by 0x8ADF74B: js::gc::ArenaLists::backgroundFinalize(js::FreeOp*, js::gc::ArenaHeader*, bool) (jsgc.cpp:1395)
==2865==    by 0x8AE3304: SweepBackgroundThings(JSRuntime*, bool) (jsgc.cpp:2207)
==2865==    by 0x8AE490F: js::GCHelperThread::doSweep() (jsgc.cpp:2489)
==2865==    by 0x8AE39A9: js::GCHelperThread::threadLoop() (jsgc.cpp:2333)
==2865==    by 0x8AE3911: js::GCHelperThread::threadMain(void*) (jsgc.cpp:2312)
==2865==    by 0x4097624: _pt_root (ptthread.c:192)
==2865==    by 0x403032F: mythread_wrapper (hg_intercepts.c:219)
==2865==    by 0x4A2CE99: start_thread (pthread_create.c:308)
==2865==    by 0xB428CBC: clone (clone.S:112)

Looks like the main thread is calling UnmarkGrayGCThing while at the same time the GC thread is calling IsMarked on the same address.
It looks like the "previous read" is the finalization of a TypeObject.
(In reply to Andrew McCreight [:mccr8] from comment #1)
> It looks like the "previous read" is the finalization of a TypeObject.

Well, inlining messes up these stacks a little, but the finalize() call is calling IsMarked here (I think!):

http://mxr.mozilla.org/mozilla-central/source/js/src/jsgc.cpp#341
Summary: Helgrind warning about data race on → Helgrind warning about data race on mark bits
My guess without knowing anything about this code would be that finalization is checking if an object's black bit is set at the same time as UnmarkGray is running.  If the object is black, that wouldn't seem like it would matter, as gray also has the black bit set.  If the object doesn't have its mark bit set, I assume it would be dead, and thus wouldn't have UnmarkGray set on it?
Yeah, this is safe. We should annotate the access in js::gc::Arena::finalize.
Assignee: general → nobody
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.