Closed Bug 1156903 Opened 9 years ago Closed 9 years ago

spike of crash in F21225463 with async plugin init

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Windows 7
defect
Not set
critical

Tracking

(firefox39+ verified, firefox40+ fixed)

RESOLVED FIXED
mozilla40
Tracking Status
firefox39 + verified
firefox40 + fixed

People

(Reporter: kairo, Assigned: bugzilla)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #813205 +++

[Tracking Requested - why for this release]:

This signature is by far the largest plugin crash on Dev Edition right now, with a really bad plugin crash rate in that 39 train atm. This is the first time we have async plugin init on, so that's likely the culprit or trigger here.

They all seem to bug EXCEPTION_ACCESS_VIOLATION_READ at 0x0.

In the small sample I looked at, I saw two different kinds of stacks in there:

bp-27292f8a-2be0-410c-9979-7763a2150420 et al:
0 	npswf32_17_0_0_169.dll 	F21225463__________________________________________________________________________________________________________ 	F209679109___________________________________________________________________________________:1237
1 	npswf32_17_0_0_169.dll 	F_1746776035_______________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________ 	F462186421___________________________________________________________:859
2 	npswf32_17_0_0_169.dll 	F_1557046174___________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________ 	F1104190711_____________________________________________________________________________:302
3 	npswf32_17_0_0_169.dll 	F1250129937___________________________________________ 	F1237873649__________________________________________________________________:16
4 	npswf32_17_0_0_169.dll 	F320803133____________________________________________ 	F_1183861151____________________________________________________________:84
5 	user32.dll 	MsgWaitForMultipleObjects 	
6 	npswf32_17_0_0_169.dll 	F_1513036030________________________________________ 	F_1654095196__________________________________________________________________:47
7 	npswf32_17_0_0_169.dll 	F_652032984_____________________________________________________ 	F_1951567228__________________________________________________________:261
8 	npswf32_17_0_0_169.dll 	F1607135317_____________________________________________________________________ 	F_851861807__________________________________________________________:134
9 	npswf32_17_0_0_169.dll 	F2166389_____________________________________________________________________ 	F_851861807__________________________________________________________:560
10 	npswf32_17_0_0_169.dll 	F_917831355____________________________________________ 	F_851861807__________________________________________________________:488
11 	npswf32_17_0_0_169.dll 	F1315696776________________________________ 	F_851861807__________________________________________________________:439
12 	npswf32_17_0_0_169.dll 	F_1428703866________________________________ 	F_766591945____________________________________________________________________:203
13 	npswf32_17_0_0_169.dll 	F845925699_____________________________________ 	F1677514683__________________________________________________________________________________:104
14 	npswf32_17_0_0_169.dll 	F15952908_________________________________________________________ 	F209679109___________________________________________________________________________________:312
15 	npswf32_17_0_0_169.dll 	F1601322143_______________________________________________________________ 	F209679109___________________________________________________________________________________:1762
16 	xul.dll 	mozilla::plugins::PluginInstanceChild::DoNPP_New() 	dom/plugins/ipc/PluginInstanceChild.cpp
17 	xul.dll 	mozilla::plugins::PluginModuleChild::RecvAsyncNPP_New(mozilla::plugins::PPluginInstanceChild*) 	dom/plugins/ipc/PluginModuleChild.cpp
18 	xul.dll 	mozilla::plugins::PPluginModuleChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PPluginModuleChild.cpp
[...]

bp-6b86cc3d-9953-42a7-809f-0b0222150420 et al:
0 	npswf32_17_0_0_169.dll 	F21225463__________________________________________________________________________________________________________ 	F209679109___________________________________________________________________________________:1237
1 	npswf32_17_0_0_169.dll 	F_1746776035_______________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________ 	F462186421___________________________________________________________:859
2 	npswf32_17_0_0_169.dll 	F_1557046174___________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________ 	F1104190711_____________________________________________________________________________:302
3 	npswf32_17_0_0_169.dll 	F1250129937___________________________________________ 	F1237873649__________________________________________________________________:16
4 	ntdll.dll 	NtWaitForSingleObject 	
5 	kernelbase.dll 	WaitForSingleObjectEx 	
6 	npswf32_17_0_0_169.dll 	F909392315_____________________________________ 	F_851861807__________________________________________________________:699
7 	npswf32_17_0_0_169.dll 	F1758486830______________________________________ 	F_611775155__________________________________________________________:90
8 	npswf32_17_0_0_169.dll 	F_917831355____________________________________________ 	F_851861807__________________________________________________________:488
9 	npswf32_17_0_0_169.dll 	F1315696776________________________________ 	F_851861807__________________________________________________________:439
10 	npswf32_17_0_0_169.dll 	F_1428703866________________________________ 	F_766591945____________________________________________________________________:203
11 	npswf32_17_0_0_169.dll 	F845925699_____________________________________ 	F1677514683__________________________________________________________________________________:104
12 	npswf32_17_0_0_169.dll 	F_1395837734_____________________________________________________ 	F209679109___________________________________________________________________________________:500
13 	npswf32_17_0_0_169.dll 	F_653069593______________________________________________________________ 	F209679109___________________________________________________________________________________:1796
14 	xul.dll 	mozilla::plugins::BrowserStreamChild::StreamConstructed(nsCString const&, bool const&, unsigned short*) 	dom/plugins/ipc/BrowserStreamChild.cpp
15 	xul.dll 	NewStreamAsyncCall::Run() 	dom/plugins/ipc/PluginInstanceChild.cpp
16 	xul.dll 	base::MessagePumpForUI::DoRunLoop() 	ipc/chromium/src/base/message_pump_win.cc
Jeromie, could you please take a look at this crash stack? It has spiked in 39, apparently due to the enabling of asynchronous plugin initialization. Is there a problem on the browser side that is causing this, or is this something that needs to be fixed on your end?
Flags: needinfo?(jeclark)
I've filed ADBE 3973470 to track this.
Flags: needinfo?(jeclark)
Tracking this plug-in crash on 39+.
Is there any progress on this on the Adobe side? Do you have any recommendations as to anything that we could do on the browser side to prevent this?
Flags: needinfo?(jeclark)
I have no new info.  I ran into some problems deobfuscating the stack which ate up a few days.  I've reassigned it, but it's in the queue still.
Flags: needinfo?(jeclark)
Attached patch Quirk FlagSplinter Review
I have learned that this crash is due to Flash omitting a return code check after calling NPN_GetValue(NPNVdocumentOrigin). It then attempts to dereference a null char* when the function fails. This patch adds a quirk where we return an empty (as opposed to null) string in this case.

The reason why this spiked with asyncInit is that it is now possible for the user to navigate away from the plugin's document while the plugin instance is still initializing. If the plugin attempts to retrieve the document origin as part of its initialization, there is no longer a content object. NPN_GetValue must then return a failure code.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Attachment #8601941 - Flags: review?(jmathies)
Comment on attachment 8601941 [details] [diff] [review]
Quirk Flag

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

::: dom/plugins/ipc/PluginInstanceChild.cpp
@@ +395,5 @@
>              return NPERR_GENERIC_ERROR;
>          }
> +        if (result == NPERR_NO_ERROR ||
> +            (GetQuirks() &
> +                PluginModuleChild::QUIRK_FLASH_RETURN_EMPTY_DOCUMENT_ORIGIN)) {

if result equals NPERR_NO_ERROR, doesn't that mean 'v' is a valid string? How can 'v' be null in this case?
Attachment #8601941 - Flags: review?(jmathies) → review-
(In reply to Jim Mathies [:jimm] from comment #7)
> if result equals NPERR_NO_ERROR, doesn't that mean 'v' is a valid string?
> How can 'v' be null in this case?

When result == NPERR_NO_ERROR, v is a valid string and is converted, which is what we want. The null problem only happens when result != NPERR_NO_ERROR. Without this patch, we don't copy v to a char* in that case. Since Flash passes in *aValue == nullptr, the crash occurs when Flash blindly goes to dereference **aValue without checking anything.
Flags: needinfo?(jmathies)
Comment on attachment 8601941 [details] [diff] [review]
Quirk Flag

Ok, and ToNewCString handles a null or empty 'v' value correctly? Returning a valid but empty string?
Flags: needinfo?(jmathies)
Attachment #8601941 - Flags: review- → review+
(In reply to Jim Mathies [:jimm] from comment #9)
> Comment on attachment 8601941 [details] [diff] [review]
> Quirk Flag
> 
> Ok, and ToNewCString handles a null or empty 'v' value correctly? Returning
> a valid but empty string?

Affirmative.
Comment on attachment 8601941 [details] [diff] [review]
Quirk Flag

Approval Request Comment
[Feature/regressing bug #]: asynchronous plugin init
[User impact if declined]: Flash crashes
[Describe test coverage new/current, TreeHerder]: Locally. Needs to be exercised on Aurora where async plugin init is enabled.
[Risks and why]: Low, simple patch
[String/UUID change made/needed]: None
Attachment #8601941 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a7d9fef3f890
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8601941 [details] [diff] [review]
Quirk Flag

Great. Approved for uplift to aurora. We should keep an eye on this over the weekend and make sure nothing is super crashy before the merge on Monday.
Attachment #8601941 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Kairo, I figure you will notice this but here's a heads up just in case you notice any flash crashes this weekend.
Flags: needinfo?(kairo)
And this definitely went down on Dev Edition after the fix, I think plugin crashes might be completely back to pre-async levels. \o/
Flags: needinfo?(kairo)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.