858800 - crash in mozilla::plugins::MiniShmBase::GetWritePtr

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Scoobidiver (away)]

It's #83 browser crasher in 20.0 and first showed up in 20.0a2/20130216. The regression range is:
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=9e4a356f693e&tochange=284d0218eb3d
It's likely a regression from bug 834127.

Signature 	mozilla::plugins::MiniShmBase::GetWritePtr<mozilla::plugins::PluginHangUICommand>(mozilla::plugins::PluginHangUICommand*&) More Reports Search
UUID	c5a8c16d-40c8-476d-9bae-2acdc2130405
Date Processed	2013-04-05 18:28:25
Uptime	3241
Last Crash	3.7 weeks before submission
Install Age	1.4 days since version was first installed.
Install Time	2013-04-04 07:47:27
Product	Firefox
Version	20.0
Build ID	20130326150557
Release Channel	release
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	AuthenticAMD family 15 model 104 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION_WRITE
Crash Address	0x6b20000
User Comments	every time i,m on castleville the computer crashes. several times.
App Notes 	
AdapterVendorID: 0x10de, AdapterDeviceID: 0x0531, AdapterSubsysID: 30cf103c, AdapterDriverVersion: 7.15.11.7967
D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers- 
Processor Notes 	sp-processor02.phx1.mozilla.com_17868:2008
EMCheckCompatibility	True
Adapter Vendor ID	0x10de
Adapter Device ID	0x0531
Total Virtual Memory	4294836224
Available Virtual Memory	3768205312
System Memory Use Percentage	29
Available Page File	2827100160
Available Physical Memory	1458847744

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::plugins::MiniShmBase::GetWritePtr<mozilla::plugins::PluginHangUICommand 	dom/plugins/ipc/hangui/MiniShmBase.h:135
1 	xul.dll 	mozilla::plugins::PluginHangUIParent::SendCancel 	dom/plugins/ipc/PluginHangUIParent.cpp:278
2 	xul.dll 	mozilla::plugins::PluginHangUIParent::Cancel 	dom/plugins/ipc/PluginHangUIParent.cpp:267
3 	xul.dll 	mozilla::plugins::PluginModuleParent::FinishHangUI 	dom/plugins/ipc/PluginModuleParent.cpp:588
4 	xul.dll 	mozilla::plugins::PluginModuleParent::ExitedCxxStack 	dom/plugins/ipc/PluginModuleParent.cpp:363
5 	xul.dll 	mozilla::ipc::RPCChannel::ExitedCxxStack 	ipc/glue/RPCChannel.cpp:616
6 	xul.dll 	mozilla::ipc::RPCChannel::CxxStackFrame::~CxxStackFrame 	obj-firefox/dist/include/mozilla/ipc/RPCChannel.h:276
7 	xul.dll 	mozilla::ipc::RPCChannel::Call 	ipc/glue/RPCChannel.cpp:185
8 	xul.dll 	mozilla::plugins::PPluginInstanceParent::CallPBrowserStreamConstructor 	obj-firefox/ipc/ipdl/PPluginInstanceParent.cpp:993
9 	xul.dll 	mozilla::plugins::PluginInstanceParent::NPP_NewStream 	dom/plugins/ipc/PluginInstanceParent.cpp:1417
10 	xul.dll 	mozilla::plugins::PluginModuleParent::NPP_NewStream 	dom/plugins/ipc/PluginModuleParent.cpp:864
11 	xul.dll 	nsNPAPIPluginStreamListener::OnStartBinding 	dom/plugins/base/nsNPAPIPluginStreamListener.cpp:326
12 	xul.dll 	nsPluginStreamListenerPeer::SetUpStreamListener 	dom/plugins/base/nsPluginStreamListenerPeer.cpp:1121
13 	xul.dll 	nsPluginStreamListenerPeer::OnStartRequest 	dom/plugins/base/nsPluginStreamListenerPeer.cpp:555
14 	xul.dll 	mozilla::net::nsHttpChannel::CallOnStartRequest 	netwerk/protocol/http/nsHttpChannel.cpp:960
15 	xul.dll 	mozilla::net::nsHttpChannel::InitCacheEntry 	netwerk/protocol/http/nsHttpChannel.cpp:3630
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Aplugins%3A%3AMiniShmBase%3A%3AGetWritePtr%3Cmozilla%3A%3Aplugins%3A%3APluginHangUICommand%3E%28mozilla%3A%3Aplugins%3A%3APluginHangUICommand*%26%29

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Crash fix

Crash fix for Firefox 20+. Depending on order of thread execution, the affected line could cause the browser-side IPC to be cleaned up prematurely, pulling the rug out from under the main thread and causing a crash.

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 734124 [details] [diff] [review]
Crash fix

I've never worked on this code, sending this over to benjamin.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Crash fix v2

I've determined that this patch should also include some increased timeouts in the IPC. The current timeouts (particularly in the child process) are lapsing on slower machines and contributing to the problem.

Comment 4

[Benjamin Smedberg]

Comment on attachment 735379 [details] [diff] [review]
Crash fix v2

I really don't know what these constants control or what unit they are in: can you add comments to them? 300 seconds seems like a long time, assuming they are in ms. If they are in us, maybe 30 seconds makes more sense ;-)

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

I'll comment the timeouts better, but I think I'll also do one better: I'll send the value of dom.ipc.plugins.timeoutSecs over to the child process and use that for IPC timeouts, since we know that value is going to be the worst-case waiting time for a hung plugin.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Crash fix v3

This revision uses the value of dom.ipc.plugins.timeoutSecs (converted to milliseconds) as the IPC timeout for the Plugin Hang UI. My rationale is that this timeout is the worst-case delay for a plugin hang, so if the UI is taking a while to spin up, the point is moot by the time this timeout elapses. Since its default is 45 seconds, I think that this is pretty reasonable.

It still includes the fix from the first revision of the patch for this bug, since that is still needed should the timeouts actually manage to lapse.

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

Try build:
https://tbpl.mozilla.org/?tree=Try&rev=3eb4183aecec

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9688317ad779

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 735979 [details] [diff] [review]
Crash fix v3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 805591
User impact if declined: Crashes on slow hardware running Windows when loading heavyweight Flash content (currently #70 on 20.0 release topcrashers)
Testing completed (on m-c, etc.): Tested using regression tests from bug 826851
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9688317ad779

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 735979 [details] [diff] [review]
Crash fix v3

Definitely appropriate for Aurora 22, will need to consider whether the criticality of this issue warrants changing plugin code on Beta.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4098c8763a3e

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Alex Keybl [:akeybl] from comment #10)
> Comment on attachment 735979 [details] [diff] [review]
> Crash fix v3
> 
> Definitely appropriate for Aurora 22, will need to consider whether the
> criticality of this issue warrants changing plugin code on Beta.

While the crash isn't a highly ranked topcrasher, I have reason to believe that the users who are seeing this are seeing it repeatedly. From what I've read in some of the crashstats comments, some users are seeing it so often that their Flash games are essentially unplayable.

Comment 13

[bhavana bajaj [:bajaj]]

Comment on attachment 735979 [details] [diff] [review]
Crash fix v3

Approving on beta after discussing this with Aaron on irc.This crash although, not high in volume appears to be annoying for users who run into it.Also, the problem is clearly understood and the fix is the definite way of resolving the issue, being low risk.

I am approving this on beta, in preparation for getting it landed for our beta 3.We will have an opportunity to react and backout if the crash is not fixed or due to any unforeseen regressions it may cause .

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/releases/mozilla-beta/rev/e512a5fc6555

Comment 15

[Manuela Muntean [Away]]

No more crash reports in Socorro, regarding last month and Firefox 22 beta cycle.

Details here: 

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=mozilla%3A%3Aplugins%3A%3AMiniShmBase%3A%3AGetWritePtr%26lt%3Bmozilla%3A%3Aplugins%3A%3APluginHangUICommand%26gt%3B%28mozilla%3A%3Aplugins%3A%3APluginHangUICommand%2A%26amp%3B%29&reason_type=contains&date=06%2F13%2F2013%2013%3A19%3A22&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Aplugins%3A%3AMiniShmBase%3A%3AGetWritePtr%3Cmozilla%3A%3Aplugins%3A%3APluginHangUICommand%3E%28mozilla%3A%3Aplugins%3A%3APluginHangUICommand%2A%26%29

Comment 16

[Manuela Muntean [Away]]

Verified fixed on Fx 23. No crashes regarding Firefox 23.0a2 and no crashes after 2013-04-15 either.

Details here:

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=mozilla%3A%3Aplugins%3A%3AMiniShmBase%3A%3AGetWritePtr%26lt%3Bmozilla%3A%3Aplugins%3A%3APluginHangUICommand%26gt%3B%28mozilla%3A%3Aplugins%3A%3APluginHangUICommand%2A%26amp%3B%29&reason_type=contains&date=06%2F13%2F2013%2013%3A19%3A22&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Aplugins%3A%3AMiniShmBase%3A%3AGetWritePtr%3Cmozilla%3A%3Aplugins%3A%3APluginHangUICommand%3E%28mozilla%3A%3Aplugins%3A%3APluginHangUICommand%2A%26%29