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)
Tracking
(firefox39+ verified, firefox40+ fixed)
RESOLVED
FIXED
mozilla40
People
(Reporter: kairo, Assigned: bugzilla)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
3.63 KB,
patch
|
jimm
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
+++ 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
Assignee | ||
Updated•9 years ago
|
Blocks: asyncplugininit
Assignee | ||
Comment 1•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
Tracking this plug-in crash on 39+.
Assignee | ||
Comment 4•9 years ago
|
||
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)
Comment 5•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
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.
Comment 7•9 years ago
|
||
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-
Assignee | ||
Comment 8•9 years ago
|
||
(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 9•9 years ago
|
||
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+
Assignee | ||
Comment 10•9 years ago
|
||
(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.
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d9fef3f890
Assignee | ||
Comment 12•9 years ago
|
||
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?
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a7d9fef3f890
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 14•9 years ago
|
||
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+
Comment 15•9 years ago
|
||
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)
Assignee | ||
Comment 16•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/c9c219b74c16
Reporter | ||
Comment 18•9 years ago
|
||
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)
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•