1191482 - crash in js::WeakValueCache<T>::sweep(js::FreeOp*)

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[alex_mayorga]

This bug was filed from the Socorro interface and is 
report bp-05f898d3-f623-463c-8b6c-ce2a32150805.
=============================================================

Had this content crash on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:42.0) Gecko/20100101 Firefox/42.0 ID:20150805030208 CSet: f3b757156f69

Unfortunately I do not have STR.

There are 20 or so other reports at https://crash-stats.mozilla.com/report/list?product=Firefox&signature=js%3A%3AWeakValueCache%3CT%3E%3A%3Asweep%28js%3A%3AFreeOp%2A%29

Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	js::WeakValueCache<unsigned int, js::ReadBarriered<js::jit::JitCode*>, js::DefaultHasher<unsigned int>, js::RuntimeAllocPolicy>::sweep(js::FreeOp*) 	js/src/jsweakcache.h
1 	xul.dll 	JSCompartment::sweepJitCompartment(js::FreeOp*) 	js/src/jscompartment.cpp
2 	xul.dll 	js::gc::GCRuntime::beginSweepingZoneGroup() 	js/src/jsgc.cpp
3 	xul.dll 	js::gc::GCRuntime::sweepPhase(js::SliceBudget&) 	js/src/jsgc.cpp
4 	xul.dll 	js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&, JS::gcreason::Reason) 	js/src/jsgc.cpp
5 	xul.dll 	js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&, JS::gcreason::Reason) 	js/src/jsgc.cpp
6 	xul.dll 	js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) 	js/src/jsgc.cpp
7 	xul.dll 	js::gc::GCRuntime::gcSlice(JS::gcreason::Reason, __int64) 	js/src/jsgc.cpp
8 	xul.dll 	nsJSContext::GarbageCollectNow(JS::gcreason::Reason, nsJSContext::IsIncremental, nsJSContext::IsShrinking, __int64) 	dom/base/nsJSEnvironment.cpp
9 	xul.dll 	nsTimerImpl::Fire() 	xpcom/threads/nsTimerImpl.cpp
10 	xul.dll 	nsTimerEvent::Run() 	xpcom/threads/TimerThread.cpp
11 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
12 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
13 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
14 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
15 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
16 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
17 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
18 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
19 	xul.dll 	XRE_RunAppShell 	toolkit/xre/nsEmbedFunctions.cpp
20 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
21 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
22 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
23 	xul.dll 	XRE_InitChildProcess 	toolkit/xre/nsEmbedFunctions.cpp
24 	plugin-container.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
25 	plugin-container.exe 	__tmainCRTStartup 	f:/dd/vctools/crt/crtw32/startup/crt0.c:255
26 	kernel32.dll 	BaseThreadInitThunk 	
27 	ntdll.dll 	RtlUserThreadStart 	
28 	kernel32.dll 	BasepReportFault 	
29 	kernel32.dll 	BasepReportFault

Comment 1

[Jon Coppeard (:jonco) (PTO until 2nd March)]

From the crash address it looks like we are calling IsAboutToBeFinalized() for a value that is a null pointer in WeakValueCache::sweep().

Comment 2

[(Away)]

This signature spiked on the August 5 build and is very likely the same root cause as bug 1191465. If the volume goes back down in tomorrow's nightly (which is expected to have a fix) then I'll resolve this.

Comment 3

[Jon Coppeard (:jonco) (PTO until 2nd March)]

Attachment: bug1191482-tidy-weak-cache-sweep

Good to know this was probably fixed by backouts.

While investigating I wrote the following patch that tidies up weak cache sweeping and adds a bunch of assertions.  It didn't catch anything but I figure it's worth landing anyway.

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8644906 [details] [diff] [review]
bug1191482-tidy-weak-cache-sweep

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

Nice!

Comment 5

[alex_mayorga]

¡Hola David!

Is this something you could please help landing?

I reviewed https://wiki.mozilla.org/BugzillaAutoLanding but it is a bit above my head =)

¡Gracias!

Comment 6

[(Away)]

Please do not set checkin-needed unless you are the patch author or have their permission. (Who knows, maybe Jon found some issues in local testing that he wants to address before landing.)

Anyway, we don't need to hurry on this patch. This patch will not fix the crashes that you are seeing. The GC crashes from last week are all related to bug 1191465 and that should be fixed starting in the next nightly.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d7b4d91def

Comment 8

[alex_mayorga]

(In reply to David Major [:dmajor] from comment #6)
> Please do not set checkin-needed unless you are the patch author or have
> their permission. (Who knows, maybe Jon found some issues in local testing
> that he wants to address before landing.)

¡Hola David!

My apologies.

So what's the right thing to do with this bug?

Bugzilla shows a warning saying "Unassigned bug with patches attached". Shall I just ignore that message?

¡Gracias!

Comment 9

[(Away)]

This bug is in kind of a weird situation because the root cause of the crashes is ultimately a duplicate of bug 1191465, but we used this bug to land some cleanup code on the side.

I wouldn't worry too much about the status of this one.

Comment 10

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

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