916860 - crash in JS_ValueToObject(JSContext*, JS::Value, JSObject**)

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Tracy Walker [:tracy]]

This bug was filed from the Socorro interface and is 
report bp-6bdc87bf-1d3e-4ba8-a792-6434e2130916.
=============================================================

This has gone explosive on Nightly builds since 20130911030258.  #11 on topcrasher for 26.0a1 


Urls' point heavily, but not exclusively, to chip.de

20 	http://www.chip.de/
4 	http://www.chip.de/?utm_source=download-sponsor&utm_medium=DSwebcheckin&utm_c...
4 	http://www.chip.de/downloads/c1_downloads_auswahl_36535523.html?t=1379022834&...
3 	http://www.chip.de/downloads/c1_downloads_auswahl_44216078.html?t=1379014114&...
3 	http://www.chip.de/artikel/35-Jahre-CHIP-35-Vollversionen-gratis-zum-Download...
3 	http://www.chip.de/downloads/c1_downloads_hs_getfile_v1_61676501.html?t=13790...
3 	about:blank
3 	http://www.chip.de/Downloads_13649224.html
2 	http://www.istockphoto.com/my_uploads.php?page=1&order=CreationDate
2 	http://www.chip.de/downloads/Nvidia-GeForce-Treiber-fuer-Windows-XP_13000370....
2 	http://www.chip.de/Downloads_13649224.html?of=0
2 	http://www.chip.de/downloads/c1_downloads_auswahl_27322639.html?t=1379336177&...
2 	http://www.chip.de/downloads/7-Taskbar-Tweaker_38804910.html
2 	http://www.istockphoto.com/my_uploads.php

Comment 1

[Tracy Walker [:tracy]]

Attachment: correlations

Comment 2

[Tracy Walker [:tracy]]

pushlog:  http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e5ca10a2b3d0&tochange=9e9f74116749

Comment 3

[Benjamin Smedberg]

On a worker, CHECK_REQUEST(cx) is crashing with address 0xdadadada - Marking s-s for now.

There's a bunch of worker stuff in the range, including:

Nikhil Marathe — Bug 901291 - Get WebIDL callbacks working on Workers. r=khuey
Jeff Walden — Bug 897678 - Make worker code stop using propertyops, as a step toward removing propertyops altogether. r=mrbkap
Boris Zbarsky — Bug 914334. Change codegen to reflect EventListener as a WebIDL callback on workers. r=khuey
Mark Hammond — Bug 913950 - close all existing ports on worker termination. r=mixedpuppy
Olli Pettay — Bug 910910 - Enable Event ctors in workers, r=khuey

Comment 4

[Jeff Walden [:Waldo]]

The crash mentioned in comment 0 has this in it:

7|0|mozjs.dll|JS_ValueToObject(JSContext *,JS::Value,JSObject * *)|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:53d5e43e23cc|372|0x31 37|1|xul.dll|mozilla::dom::workers::EventListenerManager::DispatchEvent(JSContext *,mozilla::dom::workers::EventTarget const &,JSObject *,mozilla::ErrorResult &)|hg:hg.mozilla.org/mozilla-central:dom/workers/EventListenerManager.cpp:53d5e43e23cc|390|0x2a 37|2|xul.dll|mozilla::dom::EventTargetBinding_workers::dispatchEvent|hg:hg.mozilla.org/mozilla-central:obj-firefox/dom/bindings/EventTargetBinding.cpp:53d5e43e23cc|803|0x25 37|3|xul.dll|mozilla::dom::EventTargetBinding_workers::genericMethod|hg:hg.mozilla.org/mozilla-central:obj-firefox/dom/bindings/EventTargetBinding.cpp:53d5e43e23cc|952|0x23 37|4|mozjs.dll|js::Invoke(JSContext *,JS::Value const &,JS::Value const &,unsigned int,JS::Value *,JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:53d5e43e23cc|528|0x2ae

which is suggestive of the event-mechanics-touching changes in comment 3.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Bug 914334 seems like the most likely candidate here.

Looks like we're dying inside a compartment check, not JS_CHECK_RECURSION.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> 390     if (!JS_ValueToObject(aCx, listenerVal, listenerObj.address())) {

Nothing says aCx and listenerVal are same-compartment.  I agree that bug 914334 could have caused this, maybe, depending on what exactly happened before with the XPCWrappedJS bits...

Kyle, you want me to take this, or do you want to?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Kyle points out worker should only have one compartment....  Then what gives?

Comment 9

[Nicolas B. Pierron [:nbp]]

(In reply to Boris Zbarsky [:bz] from comment #7)
> Kyle points out worker should only have one compartment....  Then what gives?

From what I understand, we have 3 compartments in a worker.  One for the atom compartment, one for the self-hosted code, and one for the user code.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Hmm.  Which one does workers::GetCurrentThreadJSContext return?

Comment 11

[Boris Zbarsky [:bzbarsky]]

But presumably WorkerRunnable::Run would get that same thing?

Though note that it sometimes uses ParentJSContext.  Could we be in that case?  Kyle?

Comment 12

[Boris Zbarsky [:bzbarsky]]

I believe the patch in bug 918450 will fix this.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Is this fixed now that bug 918450 is fixed?

Comment 14

[Tracy Walker [:tracy]]

Volume peaked on 20130918. But there continues to be enough crashes occurring with this signature to keep it in the top 5 crashers over the last 3 days of data.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Hmm.  The fix for bug 918450 would first show up in a 2013-09-21 m-c nightly.  It hasn't landed on Aurora 26 yet.

Looking at only Nightly 27 crashes, I see no crashes in a 2013-09-21 nightly or later.  So I claim this is fixed.  We just need to land bug 918450 on Aurora.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Ah, it just landed on Aurora.  So we should look at tomorrow's nightlies.

Comment 17

[Tracy Walker [:tracy]]

Cool! Yes, the volume since the 21st is on Aurora.  No crashes on Nightly in the same time frame.