Last Comment Bug 640901 - Crash [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType, bool, int*) ] [@ mozilla::dom::PAudioParent::CreateSharedMemory(unsigned int, mozilla::ipc::SharedMemory::SharedMemoryType,
: Crash [@ mozilla::plugins::PPluginIdentifierParent::CreateSharedMemory(unsign...
Status: RESOLVED FIXED
[watching crash-stats][fx4-rc-ridealong]
: crash, regression, relnote
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Windows 7
: -- critical with 1 vote (vote)
: ---
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
Mentors:
: 641218 643515 (view as bug list)
Depends on: CVE-2013-1679
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-11 01:34 PST by Scoobidiver (away)
Modified: 2013-12-27 14:34 PST (History)
12 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
Macaw+
.1-fixed


Attachments
Test (2.72 KB, patch)
2011-03-15 07:31 PDT, Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
no flags Details | Diff | Splinter Review
Patch, v1 (2.80 KB, patch)
2011-03-15 09:59 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
cjones.bugs: review-
Details | Diff | Splinter Review
Patch, v2 (6.52 KB, patch)
2011-03-18 11:16 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
no flags Details | Diff | Splinter Review
Patch, v3 (8.00 KB, patch)
2011-03-21 11:00 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
cjones.bugs: review+
Details | Diff | Splinter Review
Fixes (3.75 KB, patch)
2011-03-21 18:54 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Test for bug 640901 (10.26 KB, patch)
2011-03-21 18:54 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Test for bug 640901, v1.1 (10.26 KB, patch)
2011-03-21 18:58 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Test for bug 640901, v1.2 (9.87 KB, patch)
2011-03-21 18:59 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review

Description Scoobidiver (away) 2011-03-11 01:34:14 PST
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
Comment 1 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-11 07:55:33 PST
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?
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-11 11:10:20 PST
I have no idea what might be happening there.  Ben, is the ScriptableObject stuff doing what's supposed to?
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-03-11 11:44:40 PST
There's something wrong with the stack... Frame 1 can't possibly call frame 0 unless there's some kind of corruption.
Comment 4 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-11 12:13:06 PST
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.
Comment 5 chris hofmann 2011-03-12 08:17:42 PST
*** Bug 641218 has been marked as a duplicate of this bug. ***
Comment 6 chris hofmann 2011-03-12 08:19:56 PST
moving up the daily ranking pretty fast, #36 on thursday, #24 yesterday.

more stats, test urls, and other info in Bug 641218
Comment 7 Asa Dotzler [:asa] 2011-03-12 11:27:48 PST
carrying chofmann's nomination over from duped bug.
Comment 8 chris hofmann 2011-03-14 11:57:59 PDT
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
Comment 9 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-15 07:31:22 PDT
Created attachment 519401 [details] [diff] [review]
Test

This test reproduces the error.
Comment 10 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-03-15 09:59:40 PDT
Created attachment 519432 [details] [diff] [review]
Patch, v1

I think this should fix us. Basically calling a constructor that fails will cause the actor to be deleted, but not its subtree.
Comment 11 Mike Beltzner [:beltzner, not reading bugmail] 2011-03-15 11:00:52 PDT
Can we get a brief description of the type of user actions that would cause this crash?
Comment 12 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-15 11:09:09 PDT
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.
Comment 13 chris hofmann 2011-03-15 11:51:34 PDT
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 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-15 12:31:17 PDT
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 ;).
Comment 15 Mike Beltzner [:beltzner, not reading bugmail] 2011-03-15 15:20:52 PDT
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.
Comment 16 Mike Beltzner [:beltzner, not reading bugmail] 2011-03-16 07:14:43 PDT
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.
Comment 17 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-03-18 11:16:59 PDT
Created attachment 520245 [details] [diff] [review]
Patch, v2

How's this?
Comment 18 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-18 16:17:24 PDT
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?
Comment 19 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-03-21 11:00:20 PDT
Created attachment 520685 [details] [diff] [review]
Patch, v3
Comment 20 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-21 12:15:47 PDT
*** Bug 643515 has been marked as a duplicate of this bug. ***
Comment 21 chris hofmann 2011-03-21 13:40:52 PDT
 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 *)
Comment 22 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-21 18:53:35 PDT
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.
Comment 23 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-21 18:54:01 PDT
Created attachment 520829 [details] [diff] [review]
Fixes
Comment 24 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-21 18:54:34 PDT
Created attachment 520830 [details] [diff] [review]
Test for bug 640901

This is pushable.
Comment 25 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-21 18:58:30 PDT
Created attachment 520831 [details] [diff] [review]
Test for bug 640901, v1.1

Whups, forgot to remove some dead code from the previous version.
Comment 26 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-21 18:59:33 PDT
Created attachment 520832 [details] [diff] [review]
Test for bug 640901, v1.2

Whups, forgot to qref.  Sigh.
Comment 27 :Ehsan Akhgari 2011-03-23 19:49:44 PDT
I was trying to land this bug, but there is no way to figure out which patch(es) should be pushed.
Comment 28 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-24 11:50:47 PDT
Since it does not have checkin-needed, I wouldn't try to push it. Let the bug owners do it!
Comment 29 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-24 13:50:02 PDT
Pushed 7279fb5423bd to try.
Comment 30 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-25 10:57:18 PDT
http://hg.mozilla.org/mozilla-central/rev/6f5809f44532
http://hg.mozilla.org/mozilla-central/rev/4d230c10d507
Comment 31 christian 2011-04-11 15:33:15 PDT
Transplanted from mozilla-central onto releases/mozilla2.0:

    http://hg.mozilla.org/releases/mozilla-2.0/rev/d8239a7c9e4c
    http://hg.mozilla.org/releases/mozilla-2.0/rev/f88eb86bde40

Note You need to log in before you can comment on or make changes to this bug.