787775 - crash in js::ArrayBufferObject::removeFinalizedView

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

With combined signatures, it's #3 top crasher in today's build.
It first appeared in 18.0a1/20120901. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fcc533f691e9&tochange=a21fd4d085ad
It's likely a regression from bug 720949.

Signature 	js::ObjectImpl::getClass() More Reports Search
UUID	31fcec37-0e1b-4f96-820a-2b2db2120902
Date Processed	2012-09-02 03:05:42
Uptime	2303
Install Age	9.8 hours since version was first installed.
Install Time	2012-09-01 17:17:11
Product	Firefox
Version	18.0a1
Build ID	20120901030528
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	AuthenticAMD family 16 model 6 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
User Comments	Uploading photo to flickr, nothing else open at the time
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x0dc4, AdapterSubsysID: 087910de, AdapterDriverVersion: 8.17.12.6306
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
EMCheckCompatibility	True
Adapter Vendor ID	0x10de
Adapter Device ID	0x0dc4
Total Virtual Memory	4294836224
Available Virtual Memory	3798372352
System Memory Use Percentage	52
Available Page File	2661105664
Available Physical Memory	1024741376

Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ObjectImpl::getClass 	js/src/vm/ObjectImpl-inl.h:289
1 	mozjs.dll 	JSObject::isTypedArray 	js/src/jsobjinlines.h:793
2 	mozjs.dll 	NextViewSlot 	js/src/jstypedarray.cpp:258
3 	mozjs.dll 	js::ArrayBufferObject::removeFinalizedView 	js/src/jstypedarray.cpp:355
4 	mozjs.dll 	js::DataViewObject::obj_finalize 	js/src/jstypedarray.cpp:2448
5 	mozjs.dll 	js::gc::FinalizeArenas 	js/src/jsgc.cpp:449
6 	mozjs.dll 	js::gc::ArenaLists::queueObjectsForSweep 	js/src/jsgc.cpp:1724
7 	mozjs.dll 	BeginSweepPhase 	js/src/jsgc.cpp:3750
8 	mozjs.dll 	IncrementalCollectSlice 	js/src/jsgc.cpp:4214
9 	mozjs.dll 	GCCycle 	js/src/jsgc.cpp:4392
10 	mozjs.dll 	Collect 	js/src/jsgc.cpp:4500
11 	mozjs.dll 	js::GC 	js/src/jsgc.cpp:4524
12 	mozjs.dll 	js::ShrinkingGC 	js/src/jsfriendapi.cpp:165
13 	xul.dll 	mozilla::dom::workers::WorkerPrivate::GarbageCollectInternal 	dom/workers/WorkerPrivate.cpp:3947
14 	xul.dll 	`anonymous namespace'::GarbageCollectRunnable::WorkerRun 	dom/workers/WorkerPrivate.cpp:1385
15 	xul.dll 	mozilla::dom::workers::WorkerRunnable::Run 	dom/workers/WorkerPrivate.cpp:1796
16 	xul.dll 	mozilla::dom::workers::WorkerPrivate::DoRunLoop 	dom/workers/WorkerPrivate.cpp:2761
17 	xul.dll 	`anonymous namespace'::WorkerThreadRunnable::Run 	dom/workers/RuntimeService.cpp:331
18 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:624
19 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:257
20 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:395
21 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:90
22 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314
23 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292
24 	kernel32.dll 	BaseThreadInitThunk 	
25 	ntdll.dll 	__RtlUserThreadStart 	
26 	ntdll.dll 	_RtlUserThreadStart 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=JSObject%3A%3AisTypedArray%28%29
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AObjectImpl%3A%3AgetClass%28%29
https://crash-stats.mozilla.com/report/list?signature=js%3A%3AArrayBufferObject%3A%3AremoveFinalizedView

Comment 1

[Scoobidiver (away)]

STR:
1. Load http://apps.playcanvas.com/playcanvas/scifi/latest
=> bp-7a56c3b3-c7d6-489f-bfca-9c66c2120903

Comment 2

[Yury Delendik (:yury)]

is bug 788071 a dup?

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We're hitting this crash 100% in the gaia Gallery app, for b2g.  (On device.)  Seeing if I can reproduce in a b2g-desktop build.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Sure can!

STR
 (1) Follow the instructions here https://wiki.mozilla.org/Gaia/Hacking#Building_B2G to set up a desktop-b2g build along with gaia
 (2) Open the "Gallery" app
 (3) Wait for GC

Seems to be 100% reproducible.

Comment 6

[(not currently active) Ted Mielczarek]

I hit this crash just a few minutes ago:
https://crash-stats.mozilla.com/report/index/bp-d6c07a86-8082-4ac9-a405-92ba22120905

I believe I had loaded this WebGL demo at the time:
http://patapom.com/topics/WebGL/cathedral/intro.html

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: Register DataViews with their ArrayBuffers upon creation

I neglected to register DataViews with their ArrayBuffers, so it crashes when looking for the view in the ArrayBuffer's registered view list.

In attempting to write a test, I found multiple unimplemented bits of DataView functionality: there's no JSAPI to create them, and they cannot be clone via structured clone. The former prevents a jsapi-test, the latter a js test, and the structured clone depends on either the JSAPI entry or transferability implemented in structured cloning. There is an implementation of this in bug 720083, but it hasn't landed yet and I didn't want the dependency here.

Unfortunately, the JSAPI call JS_NewDataView() would require reimplementing all of the funky typed array cross-compartment creation logic. Which is some really ugly repetition, which should really be done by making an ArrayBufferViewObject base class, which requires making the typed arrays descend from JSObject in the first place...

So I just manually tested D.E.M.O. before and after and verified that this patch fixed it.

Advance warning: I may need to back out all of these related typed array changes (in particular, the transferable stuff and everything that depends on it) for perf regressions unless I can some up with a way of fixing it quickly.

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 658574 [details] [diff] [review]
Register DataViews with their ArrayBuffers upon creation

There are several unrelated blobs in this patch.  r+ on the blob that adds the call to addView and the test in dataview.js; for the rest, could you use a separate patch?

Comment 9

[Steve Fink [:sfink] [:s:]]

(In reply to Luke Wagner [:luke] from comment #8)
> Comment on attachment 658574 [details] [diff] [review]
> Register DataViews with their ArrayBuffers upon creation
> 
> There are several unrelated blobs in this patch.  r+ on the blob that adds
> the call to addView and the test in dataview.js; for the rest, could you use
> a separate patch?

Oh. Sorry, I attached the wrong patch!

The only change I am proposing to land right now is the single line that adds the call the addView. The test is currently useless because there's no way to neuter a DataView via either JS or the JSAPI. I will add it back in when the necessary functionality is available.

Comment 10

[Jesse Ruderman]

I hit this crash on Mac, bp-c115420f-1636-4939-b4bc-2fd5b2120905

1. Load http://maps.google.com/?vector=1 (Google Maps with WebGL)
2. Zoom in all the way to street view.
3. Pan around for a few seconds.

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This is blocking gallery testing completely for b2g/gaia.

Comment 13

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/22e9cbb8bb92

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/22e9cbb8bb92

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Test from bug 787709.
https://hg.mozilla.org/mozilla-central/rev/e2fe8fa1f29b