791798 - crash in mozilla::plugins::parent::_releaseobject

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-8fa54c97-2086-4e19-a9c4-15b622120917 .
============================================================= 

Seen while looking at Aurora crash stats.  https://crash-stats.mozilla.com/report/list?signature=mozilla::plugins::parent::_releaseobject%28NPObject*%29

The crash has been around in smaller numbers in earlier versions but has increased in Aurora. Unfortunately we are not catching the flash version that may be involved.

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::plugins::parent::_releaseobject 	dom/plugins/base/nsNPAPIPlugin.cpp:1460
1 	xul.dll 	DelayedReleaseGCCallback 	dom/plugins/base/nsJSNPRuntime.cpp:213
2 	xul.dll 	XPCJSRuntime::GCCallback 	js/xpconnect/src/XPCJSRuntime.cpp:727
3 	mozjs.dll 	Collect 	js/src/jsgc.cpp:4505
4 	mozjs.dll 	js::GCSlice 	js/src/jsgc.cpp:4538
5 	mozjs.dll 	js::NotifyDidPaint 	js/src/jsfriendapi.cpp:838
6 	xul.dll 	mozilla::layers::LayerManagerOGL::EndEmptyTransaction 	gfx/layers/opengl/LayerManagerOGL.cpp:393
7 	xul.dll 	nsXPConnect::NotifyDidPaint 	js/xpconnect/src/nsXPConnect.cpp:2754
8 	xul.dll 	PresShell::Paint 	layout/base/nsPresShell.cpp:5293
9 	xul.dll 	nsViewManager::Refresh 	view/src/nsViewManager.cpp:369
10 	xul.dll 	nsViewManager::PaintWindow 	view/src/nsViewManager.cpp:709
11 	xul.dll 	nsView::PaintWindow 	view/src/nsView.cpp:1034
12 	xul.dll 	nsWindow::OnPaint 	widget/windows/nsWindowGfx.cpp:534

Comment 1

[Benjamin Smedberg]

This is probably related to bug 790826.

Comment 2

[Scoobidiver (away)]

It first appeared in 17.0a1/20120730. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=29bff59d3bbe&tochange=36c30260e7fa
It might be a regression from bug 744121.

Comment 3

[Benjamin Smedberg]

according to the signature summary I see a crash on each of the 2012082804 and 2012082903 builds; nothing in the regression range from comment 2 looks remotely suspicious. I suspect that http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8af6a22827ec&tochange=8af2ff9c6018 or even a day earlier is more likely, and that this is likely a GC hazard somewhere in the plugin code, so we should be looking at both the JS/GC pushes as well as plugin pushes.

Comment 4

[Scoobidiver (away)]

It's #28 top browser crasher in 17.0a2 and #89 in 18.0a1.

Comment 5

[Alex Keybl [:akeybl]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> so we should be looking at both the JS/GC pushes as well as
> plugin pushes.

Tracking for FF17 and CC'ing some JS folks. Also starting this bug off with you bsmedberg.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

This is currently at #25 in 17 topcrashers, could rise more once we're on Beta -- Benjamin have you had a chance to look into this more?

Comment 8

[Benjamin Smedberg]

Attachment: Keep sDelayedReleases safe, rev. 1

gfritzsche found this while working on something else, and I'm going to mark r+ here before checking it in! Woot.

Comment 9

[Benjamin Smedberg]

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

Comment 10

[Ed Morley [:emorley]]

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

Comment 11

[Benjamin Smedberg]

Comment on attachment 670095 [details] [diff] [review]
Keep sDelayedReleases safe, rev. 1

Crashes disappeared on 12-Oct for nightly builds, woot.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Not sure, probably incremental GC.
User impact if declined: More crashes
Testing completed (on m-c, etc.): on nightly for a weekend, verified fix via crash-stats and some manual testing
Risk to taking this patch (and alternatives if risky): This patch appears quite safe to me; it's the equivalent of a null-check. The only real risk is a performance loss due to enumerating sDelayedReleases, but that shouldn't be a big array usually.
String or UUID changes made by this patch: none

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 670095 [details] [diff] [review]
Keep sDelayedReleases safe, rev. 1

Getting this landed early should give us time to gather feedback if there is any performance issue requiring this to be backed out.

Comment 13

[Benjamin Smedberg]

https://hg.mozilla.org/releases/mozilla-beta/rev/2d1dc4ef632f
https://hg.mozilla.org/releases/mozilla-aurora/rev/88c7703111d1

Comment 14

[u279076]

Scoobidiver, would you mind verifying if this is fixed for Firefox 17 and 18 as well? Thanks.

Comment 15

[Scoobidiver (away)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #14)
> Scoobidiver, would you mind verifying if this is fixed for Firefox 17 and 18
> as well? Thanks.
There are no crashes in 17.0b2 and above and after 18.0a2/20121015.