Closed Bug 640901 Opened 14 years ago Closed 14 years ago

Crash [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) ] [@ mozilla::dom::PAudioParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType,

Categories

(Core Graveyard :: Plug-ins, defect)

x86
Windows 7
defect
Not set
critical

Tracking

(blocking2.0 Macaw+, status2.0 .1-fixed)

RESOLVED FIXED
Tracking Status
blocking2.0 --- Macaw+
status2.0 --- .1-fixed

People

(Reporter: scoobidiver, Assigned: bent.mozilla)

References

Details

(Keywords: crash, regression, relnote, Whiteboard: [watching crash-stats][fx4-rc-ridealong])

Crash Data

Attachments

(4 files, 4 obsolete files)

It is a new crash signature that first appeared in 4.0 RC1. It starts showing up as #38 top crasher in 4.0 RC1. Every comments talk about opening PDF with Acrobat Reader X. Signature mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) UUID 4429fd9a-fe37-4fb4-a21b-575cf2110310 Time 2011-03-10 19:16:41.509271 Uptime 1656 Install Age 87925 seconds (1.0 days) since version was first installed. Product Firefox Version 4.0 Build ID 20110303194838 Branch 2.0 OS Windows NT OS Version 6.1.7601 Service Pack 1 CPU x86 CPU Info GenuineIntel family 6 model 37 stepping 2 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x1 User Comments Adobe Reader crashed, but this should not cause Firefox to crash! Add-ons and plugins should be in seperate processes, if they are not already. App Notes AdapterVendorID: 8086, AdapterDeviceID: 0042, AdapterDriverVersion: 8.15.10.2202 D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ Frame Module Signature [Expand] Source 0 xul.dll mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory obj-firefox/ipc/ipdl/PLayersChild.cpp:385 1 xul.dll mozilla::plugins::PPluginInstanceParent::RemoveManagee obj-firefox/ipc/ipdl/PPluginInstanceParent.cpp:2061 2 xul.dll mozilla::plugins::PPluginScriptableObjectParent::OnMessageReceived obj-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp:613 3 xul.dll mozilla::plugins::PPluginModuleParent::OnMessageReceived obj-firefox/ipc/ipdl/PPluginModuleParent.cpp:528 4 xul.dll mozilla::ipc::AsyncChannel::OnDispatchMessage ipc/glue/AsyncChannel.cpp:262 5 xul.dll mozilla::ipc::RPCChannel::Call ipc/glue/RPCChannel.cpp:244 6 xul.dll mozilla::plugins::PPluginScriptableObjectParent::CallInvalidate obj-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp:119 7 xul.dll mozilla::plugins::PluginScriptableObjectParent::ScriptableInvalidate dom/plugins/PluginScriptableObjectParent.cpp:117 8 xul.dll NPObjWrapperPluginDestroyedCallback modules/plugin/base/src/nsJSNPRuntime.cpp:1925 9 xul.dll PL_DHashTableEnumerate obj-firefox/xpcom/build/pldhash.c:754 10 xul.dll nsJSNPRuntime::OnPluginDestroy modules/plugin/base/src/nsJSNPRuntime.cpp:1992 11 xul.dll nsNPAPIPluginInstance::InitializePlugin modules/plugin/base/src/nsNPAPIPluginInstance.cpp:426 12 xul.dll nsNPAPIPluginInstance::Initialize modules/plugin/base/src/nsNPAPIPluginInstance.cpp:150 13 xul.dll nsPluginHost::TrySetUpPluginInstance modules/plugin/base/src/nsPluginHost.cpp:1383 14 xul.dll nsPluginHost::SetUpPluginInstance modules/plugin/base/src/nsPluginHost.cpp:1264 More reports at: https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=mozilla%3A%3Aplugins%3A%3APPluginIdentifierParent%3A%3ACreateSharedMemory%28unsigned%20int%2C%20mozilla%3A%3Aipc%3A%3ASharedMemory%3A%3ASharedMemoryType%2C%20bool%2C%20int*%29
cjones, if you're done with Fennec blockers could you look? It appears that a plugin returned a failure code from NPP_New, but then we end up in the weeds?
I have no idea what might be happening there. Ben, is the ScriptableObject stuff doing what's supposed to?
There's something wrong with the stack... Frame 1 can't possibly call frame 0 unless there's some kind of corruption.
I believe what's happening is: nsNPAPIPluginInstance::InitializePlugin calls PluginModuleParent::NPP_New either acrobat is crashing or NPP_New is returning a failure NPError code which ends up at either http://hg.mozilla.org/mozilla-central/annotate/4512b571e48e/dom/plugins/PluginModuleParent.cpp#l880 or #l891 At that point, the plugin has already created NPObjects. We proceed to destroy those actors, and then the PPluginInstance (or it is already destroyed), but somehow the ParentNPObject here doesn't realize that and is referencing a dead actor.
moving up the daily ranking pretty fast, #36 on thursday, #24 yesterday. more stats, test urls, and other info in Bug 641218
carrying chofmann's nomination over from duped bug.
blocking2.0: --- → ?
Whiteboard: [watching crash-stats]
update on stats mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory.unsigned.int,.mozilla::ipc::SharedMemory::SharedMemoryType,.bool,.int.. date total breakdown by build crashes count build, count build, ... 20110310 67 4.02011030319 67 , 20110311 156 4.02011030319 156 , 20110312 131 4.02011030319 131 , 20110313 201 4.02011030319 201 , still a long tail of sites where this is seen. 2 http://www.tepco.co.jp/images/tokyo.pdf 2 http://www.pizarreno.cl/upload/pizarreno/2008616123440_siding.pdf 2 http://dreamprogram.com/wp-content/uploads/2010/04/DreamToDestiny-Introduction-1.pdf 1 https://www.bestsecret.com/medias/sys_master/8487611646533440.pdf 1 https://ssl.fimmg.org/privacy/documenti/6466/20110313180802-6466.pdf 1 https://kundenshop.1und1.de/modules/frontend-consumer/pdf/handbuch_fritzbox_fon_wlann_7390.pdf 1 https://jobs.utah.gov/ui/Forms/form682.pdf
Attached patch Test (deleted) — Splinter Review
This test reproduces the error.
Attached patch Patch, v1 (obsolete) (deleted) — Splinter Review
I think this should fix us. Basically calling a constructor that fails will cause the actor to be deleted, but not its subtree.
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Attachment #519432 - Flags: review?(jones.chris.g)
Can we get a brief description of the type of user actions that would cause this crash?
Clicking on a link to a PDF file is the known cause. In this case, Acrobat is crashing (we don't know why), and we have a bug where if a plugin crashes while you are creating a plugin instance, and it has created any scriptable objects, then we crash in this way.
searching though module correlation data nothing sands out. I did notice avast! Antivirus (snxhk.dll) showing up in several reports but its just a bit more than the usual pct. maybe some combination of other incompatible modules that adds up 100%? 19% (49/255) vs. 12% (9532/78012) snxhk.dll 19% (48/255) vs. 12% (8975/78012) dhcpcsvc6.DLL 18% (47/255) vs. 8% (6148/78012) cryptdll.dll 18% (45/255) vs. 9% (6963/78012) sensapi.dll 17% (44/255) vs. 11% (8881/78012) SensApi.dll 17% (44/255) vs. 5% (4006/78012) cabinet.dll 15% (39/255) vs. 9% (6842/78012) ATL80.dll 15% (39/255) vs. 3% (2090/78012) googletoolbar-ff4.dll 15% (39/255) vs. 3% (2090/78012) frozen.dll 15% (38/255) vs. 9% (6723/78012) GrooveUtil.DLL 15% (38/255) vs. 9% (6723/78012) GrooveNew.DLL 15% (38/255) vs. 9% (6720/78012) GrooveShellExtensions.dll 14% (36/255) vs. 5% (4261/78012) msi.dll 12% (31/255) vs. 4% (2759/78012) wuapi.dll 12% (30/255) vs. 6% (4321/78012) wevtapi.dll 12% (30/255) vs. 6% (4321/78012) Wpc.dll 11% (29/255) vs. 6% (4743/78012) ntdsapi.dll 11% (29/255) vs. 5% (4098/78012) browseui.dll 10% (25/255) vs. 5% (3643/78012) GrooveIntlResource.dll 9% (22/255) vs. 4% (2791/78012) MpOAV.dll
Comment on attachment 519432 [details] [diff] [review] Patch, v1 >diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py >--- a/ipc/ipdl/ipdl/lower.py >+++ b/ipc/ipdl/ipdl/lower.py >- >+ # make sure that we aren't still hanging around in the manager's hash >+ if ptype.isManaged(): This is unnecessary. > def failCtorIf(self, md, cond): > actorvar = md.actorDecl().var() >+ type = md.decl.type.constructedType() The dtorEpilogue() helper is doing exactly this already. These two should share this logic, factoring it into a new helper would be fine. The only difference between the two is that dtorEpilogue() dispatches ActorDestroy(why==Deletion), and failed constructors are dispatching with why==AbnormalShutdown. That's not what we want; it specifically means crashes. Instead, let's add FailedConstructor to enum ActorDestroyReason, dispatch that to the actor whose constructor failed, and then dispatch AncestorDeletion to managees. This is a fundamental semantic change, you need tests ;).
Attachment #519432 - Flags: review?(jones.chris.g) → review-
We've asked :cww to get in touch with some users who are suffering from this crash to see if we can determine why it's not universally reproducible.
The early feedback from users indicates: - doesn't happen all the time - they installed Adobe Reader X the "moment it was available" - they have been running betas for a while No observable pattern with plugins, add-ons or graphics drivers.
blocking2.0: ? → .x+
Keywords: relnote
Whiteboard: [watching crash-stats] → [watching crash-stats][fx4-rc-ridealong]
Attached patch Patch, v2 (obsolete) (deleted) — Splinter Review
How's this?
Attachment #519432 - Attachment is obsolete: true
Attachment #520245 - Flags: review?(jones.chris.g)
Comment on attachment 520245 [details] [diff] [review] Patch, v2 Looks better, thanks. >diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h > enum ActorDestroyReason { > Deletion, > AncestorDeletion, > NormalShutdown, >- AbnormalShutdown >+ AbnormalShutdown, >+ FailedCtor > }; Tiny nits: s/FailedCtor/FailedConstructor/, and I'd prefer to list FailedConstructor before Deletion here. Kinda hard to explain; there's an ordering of these in my head, in which FailedConstructor goes first :S. >diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py >+ # make sure that we aren't still hanging around in the manager's hash Nit: this isn't "making sure we've been removed", which implies the code is the backstop for something that may or may not have happened by now. Instead, this code is *the* entity responsible for unregistering the actor's ID. I'm not sure you really need a python comment here given the C++ one just below it, the wording of which I prefer. >+ if type.isManager(): >+ failif.addifstmts([ >+ StmtExpr(self.callActorDestroy( >+ actorvar, why=_DestroyReason.FailedCtor)), >+ StmtExpr(self.callDeallocSubtree(md, actorvar)) > ]) This is wrong, because leaf protocols still need ActorDestroy() to be called on them. DeallocSubtree() is a no-op for leaf protocols. But see below. >+ failif.addifstmts([ >+ StmtExpr(self.callRemoveActor(actorvar, ipdltype=type)), >+ StmtReturn(ExprLiteral.NULL), >+ ]) This code is still doing exactly the same thing as |dtorEpilogue()|; why not save pain and share the impl? As we discussed on IRC, dtorEpilogue() is now doing an |unregisterActor()| made redundant by the changes to DestroySubtree(). Also, this version didn't address "dispatch [FailedConstructor] to the actor whose constructor failed, and then dispatch AncestorDeletion to managees." If we don't do that, there won't be a way for managees to distinguish their own constructor failing from an ancestor's constructor failing after the descendent was successfully constructed. See the code that munges Deletion->AncestorDeletion at http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/ipdl/lower.py#2967; we need another case for FailedConstructor->AncestorDeletion. I don't think descendents really need to care about failed ctors of ancestors, so re-using AncestorDeletion ought to be fine to dispatch to descendents' ActorDestroy()s (instead of, say, creating a new AncestorFailedConstructor destroy reason). How's the test coming along? Can I help?
Attachment #520245 - Flags: review?(jones.chris.g)
Attached patch Patch, v3 (deleted) — Splinter Review
Attachment #520245 - Attachment is obsolete: true
Attachment #520685 - Flags: review?(jones.chris.g)
Bug 643515 adds these stacks and signature. ....Signature number:183- mozilla::dom::PAudioParent::CreateSharedMemoryunsignedint,mozilla::ipc::SharedMemory::SharedMemoryType,bool,int ______ distribution of 25 different stacks, looking at top 10 frames 24 stacks like 0|0|xul.dll|mozilla::dom::PAudioParent::CreateSharedMemory(unsigned int,mozilla::ipc::SharedMemory::SharedMemoryType,bool,int *) 0|1|xul.dll|mozilla::plugins::PPluginInstanceParent::RemoveManagee(int,mozilla::ipc::RPCChannel::RPCListener *) 0|2|xul.dll|mozilla::plugins::PPluginScriptableObjectParent::OnMessageReceived(IPC::Message const &) 0|3|xul.dll|mozilla::plugins::PPluginModuleParent::OnMessageReceived(IPC::Message const &) 0|4|xul.dll|mozilla::ipc::AsyncChannel::OnDispatchMessage(IPC::Message const &) 0|5|xul.dll|mozilla::ipc::RPCChannel::Call(IPC::Message *,IPC::Message *) 0|6|xul.dll|mozilla::plugins::PPluginScriptableObjectParent::CallInvalidate() 0|7|xul.dll|mozilla::plugins::PluginScriptableObjectParent::ScriptableInvalidate(NPObject *) 0|8|xul.dll|NPObjWrapperPluginDestroyedCallback 0|9|xul.dll|PL_DHashTableEnumerate 1 stacks like 0|0|xul.dll|mozilla::dom::PAudioParent::CreateSharedMemory(unsigned int,mozilla::ipc::SharedMemory::SharedMemoryType,bool,int *) 0|1|xul.dll|mozilla::plugins::PPluginInstanceParent::RemoveManagee(int,mozilla::ipc::RPCChannel::RPCListener *) 0|2|xul.dll|mozilla::plugins::PPluginScriptableObjectParent::OnMessageReceived(IPC::Message const &) 0|3|xul.dll|mozilla::plugins::PPluginModuleParent::OnMessageReceived(IPC::Message const &) 0|4|xul.dll|mozilla::ipc::AsyncChannel::OnDispatchMessage(IPC::Message const &) 0|5|xul.dll|mozilla::ipc::RPCChannel::OnMaybeDequeueOne() 0|6|xul.dll|MessageLoop::RunTask(Task *) 0|7|xul.dll|MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &) 0|8|xul.dll|MessageLoop::DoWork() 0|9|xul.dll|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *)
Summary: Crash [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) ] → Crash [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) ] [@ mozilla::dom::PAudioParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType,
Comment on attachment 520685 [details] [diff] [review] Patch, v3 Looks good. There are some problems with this patch, but r=me with the upcoming fixes tacked on.
Attachment #520685 - Flags: review?(jones.chris.g) → review+
Attached patch Test for bug 640901 (obsolete) (deleted) — Splinter Review
This is pushable.
Attached patch Test for bug 640901, v1.1 (obsolete) (deleted) — Splinter Review
Whups, forgot to remove some dead code from the previous version.
Attachment #520830 - Attachment is obsolete: true
Attached patch Test for bug 640901, v1.2 (deleted) — Splinter Review
Whups, forgot to qref. Sigh.
Attachment #520831 - Attachment is obsolete: true
I was trying to land this bug, but there is no way to figure out which patch(es) should be pushed.
Since it does not have checkin-needed, I wouldn't try to push it. Let the bug owners do it!
Pushed 7279fb5423bd to try.
blocking2.0: .x+ → ?
blocking2.0: ? → Macaw+
Crash Signature: [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) ]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: