crash in nsNPAPIPluginInstance::RedrawPlugin on ICS and JB

RESOLVED FIXED in Firefox 15

Status

()

Core
Plug-ins
--
critical
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Scoobidiver (away), Assigned: snorp)

Tracking

(4 keywords)

15 Branch
mozilla17
ARM
Android
crash, qawanted, regression, topcrash
Points:
---

Firefox Tracking Flags

(firefox15+ fixed, firefox16+ fixed)

Details

(Whiteboard: [native-crash], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
It first appeared in 17.0a1/20120721041038. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3a05d298599e&tochange=446b788ab99d
It's likely a regression from bug 687267.

Signature 	nsNPAPIPluginInstance::RedrawPlugin More Reports Search
UUID	1763e40b-f62e-4d83-bfab-4adcc2120722
Date Processed	2012-07-22 06:28:12
Uptime	664
Last Crash	5.0 weeks before submission
Install Age	9.4 hours since version was first installed.
Install Time	2012-07-21 21:03:01
Product	FennecAndroid
Version	17.0a1
Build ID	20120721041038
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux 2.6.39.4+ #1 SMP PREEMPT Fri May 11 18:57:07 CST 2012 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'NVIDIA Corporation -- NVIDIA Tegra 3 -- OpenGL ES 2.0 14.01002 -- Model: A700, Product: a700_emea_de, Manufacturer: Acer, Hardware: picasso_mf'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
Acer A700
acer/a700_emea_de/picasso_mf:4.0.4/IMM76D/1336732816:user/release-keys
EMCheckCompatibility	True
Adapter Vendor ID	NVIDIA Corporation
Adapter Device ID	NVIDIA Tegra 3

Frame 	Module 	Signature 	Source
0 	libxul.so 	nsNPAPIPluginInstance::RedrawPlugin 	dom/plugins/base/nsNPAPIPluginInstance.cpp:802
1 	libxul.so 	nsRunnableMethodImpl<void , true>::Run 	nsThreadUtils.h:349
2 	libxul.so 	nsSurfaceTexture::NotifyFrameAvailable 	gfx/thebes/nsSurfaceTexture.cpp:258
3 	libxul.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	widget/android/AndroidJNI.cpp:1036
4 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_onSurfaceTextureFrameAvailable 	mozglue/android/APKOpen.cpp:326
5 	libdvm.so 	libdvm.so@0x1ec32 	
6 	dalvik-heap (deleted) 	dalvik-heap @0x9533ce 	
7 	libdvm.so 	libdvm.so@0x58eed 	
8 	data@app@org.mozilla.fennec-1.apk@classes.dex 	data@app@org.mozilla.fennec-1.apk@classes.dex@0x136a24 	
9 	libmozglue.so 	Java_org_mozilla_gecko_GeckoAppShell_getNextMessageFromQueue 	mozglue/android/APKOpen.cpp:325
10 		@0x5ebad5fe 	
11 	libxul.so 	nsCOMPtr_base::assign_with_AddRef 	obj-firefox/xpcom/build/nsCOMPtr.cpp:49
12 	libxul.so 	nsEventListenerManager::HandleEventInternal 	nsCOMPtr.h:614 

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsNPAPIPluginInstance%3A%3ARedrawPlugin
I can't seem to reproduce, adding qawanted for STR. I do have a plausible fix that I will post, however.
Assignee: nobody → snorp
Keywords: qawanted
Created attachment 645488 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin

Comment 3

5 years ago
Comment on attachment 645488 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin

If this check wasn't required before then it seems like it shouldn't be required now. Can you explain why it is required now? If not then I suspect it's just hiding a much worse problem and I'd rather not do it, especially without an ifdef making it android-only. These things can seem like harmless safety checks that are hard to argue with but they end up covering up deeper issues.
(Reporter)

Updated

5 years ago
Summary: crash in nsNPAPIPluginInstance::RedrawPlugin on ICS → crash in nsNPAPIPluginInstance::RedrawPlugin on ICS and JB
(In reply to Josh Aas (Mozilla Corporation) from comment #3)
> Comment on attachment 645488 [details] [diff] [review]
> Avoid crash when trying to redraw a destroyed plugin
> 
> If this check wasn't required before then it seems like it shouldn't be
> required now. Can you explain why it is required now? If not then I suspect
> it's just hiding a much worse problem and I'd rather not do it, especially
> without an ifdef making it android-only. These things can seem like harmless
> safety checks that are hard to argue with but they end up covering up deeper
> issues.

Indeed, better patch coming.
Created attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android
Attachment #645756 - Flags: review?(joshmoz)
Attachment #645488 - Attachment is obsolete: true

Comment 6

5 years ago
Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

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

Looks good, thanks!
Attachment #645756 - Flags: review?(joshmoz) → review+
(Reporter)

Updated

5 years ago
status-firefox15: --- → affected
status-firefox16: --- → affected
Version: 17 Branch → 15 Branch
(Reporter)

Comment 7

5 years ago
It's #1 top crasher in today's Nightly and Aurora.
tracking-fennec: --- → ?
Keywords: topcrash
https://hg.mozilla.org/integration/mozilla-inbound/rev/873bd2f652b7
I'll ask for Aurora and Beta approval as soon as we're sure this patch fixes it on Nightly.
https://hg.mozilla.org/mozilla-central/rev/873bd2f652b7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Updated

5 years ago
tracking-firefox15: --- → +
tracking-firefox16: --- → +

Comment 11

5 years ago
This signature completely stopped on 17.0a1 after the 2012072605 build ID, I think the patch is ready for uplift to Aurora and Beta, where this is the topcrash right now.
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #11)
> This signature completely stopped on 17.0a1 after the 2012072605 build ID, I
> think the patch is ready for uplift to Aurora and Beta, where this is the
> topcrash right now.

That's fantastic news - yes, let's move forward with the uplift (preferably before tomorrow's beta build).
Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Attachment #645756 - Flags: approval-mozilla-beta?
Attachment #645756 - Flags: approval-mozilla-aurora?
(In reply to Naoki Hirata :nhirata from comment #13)
> Bug caused by (feature/regressing bug #): 
> Risk to taking this patch (and alternatives if risky): 

Before approving, we'll still need these filled out by snorp.
(In reply to Naoki Hirata :nhirata from comment #13)
> Comment on attachment 645756 [details] [diff] [review]
> Avoid crash when trying to redraw a destroyed plugin on Android
> 
> [Approval Request Comment]
Fixes top crash, low risk.
Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

Please land to branches, before tomorrow morning PT if possible, so we can get the most beta user data on the crashes.
Attachment #645756 - Flags: approval-mozilla-beta?
Attachment #645756 - Flags: approval-mozilla-beta+
Attachment #645756 - Flags: approval-mozilla-aurora?
Attachment #645756 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-beta/rev/d37e5476fb73
https://hg.mozilla.org/releases/mozilla-aurora/rev/4853c37d2229
(Reporter)

Updated

5 years ago
status-firefox15: affected → fixed
status-firefox16: affected → fixed
Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

[Triage Comment]
Based upon the latest crash volume, we've decided to take this fix in a mobile-only 14.0.2. Please land on mozilla-release as soon as possible.
Attachment #645756 - Flags: approval-mozilla-release+
Comment on attachment 645756 [details] [diff] [review]
Avoid crash when trying to redraw a destroyed plugin on Android

sorry, wrong JB bug.
Attachment #645756 - Flags: approval-mozilla-release+
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.