Closed
Bug 852164
Opened 8 years ago
Closed 7 years ago
crash in mozilla::plugins::PluginModuleParent::TerminateChildProcess @ PL_DHashTableOperate
Categories
(Core :: Plug-ins, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla28
People
(Reporter: scoobidiver, Assigned: aklotz)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(2 files, 1 obsolete file)
1.02 KB,
patch
|
benjamin
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
5.18 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
It's #72 top browser crasher in 20.0b5 and occurs mainly on Windows XP. Signature PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCString> >::PutEntry(nsACString_internal const&) More Reports Search UUID 53e07183-695e-405e-9190-299632130318 Date Processed 2013-03-18 14:15:46 Uptime 1923 Last Crash 2.1 weeks before submission Install Age 36.1 minutes since version was first installed. Install Time 2013-03-18 13:38:59 Product Firefox Version 20.0 Build ID 20130313170052 Release Channel beta OS Windows NT OS Version 6.1.7601 Service Pack 1 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 23 stepping 10 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0xd App Notes AdapterVendorID: 0x8086, AdapterDeviceID: 0x2a42, AdapterSubsysID: 17831854, AdapterDriverVersion: 8.15.10.2555 D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ Processor Notes sp-processor08.phx1.mozilla.com_4808:2008 EMCheckCompatibility True Adapter Vendor ID 0x8086 Adapter Device ID 0x2a42 Total Virtual Memory 2147352576 Available Virtual Memory 1108848640 System Memory Use Percentage 64 Available Page File 3495723008 Available Physical Memory 1126715392 Frame Module Signature Source 0 xul.dll PL_DHashTableOperate obj-firefox/xpcom/build/pldhash.cpp:578 1 xul.dll nsTHashtable<nsBaseHashtableET<nsCStringHashKey,nsCString> >::PutEntry obj-firefox/dist/include/nsTHashtable.h:170 2 xul.dll mozilla::plugins::PluginModuleParent::TerminateChildProcess dom/plugins/ipc/PluginModuleParent.cpp:491 3 xul.dll mozilla::plugins::PluginHangUIParent::RecvUserResponse dom/plugins/ipc/PluginHangUIParent.cpp:296 4 xul.dll mozilla::plugins::PluginHangUIParent::OnMiniShmEvent dom/plugins/ipc/PluginHangUIParent.cpp:351 5 xul.dll mozilla::plugins::MiniShmBase::OnEvent dom/plugins/ipc/hangui/MiniShmBase.h:232 6 xul.dll mozilla::plugins::MiniShmParent::OnEvent dom/plugins/ipc/MiniShmParent.cpp:199 7 xul.dll mozilla::plugins::MiniShmBase::SOnEvent dom/plugins/ipc/hangui/MiniShmBase.h:299 8 ntdll.dll RtlpTpWaitCallback 9 ntdll.dll TppWaitpExecuteCallback 10 ntdll.dll TppCallbackCheckThreadBeforeCallback 11 kernel32.dll BaseThreadInitThunk 12 ntdll.dll __RtlUserThreadStart 13 ntdll.dll _RtlUserThreadStart More reports at: https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate+|+nsTHashtable%3CnsBaseHashtableET%3CnsCStringHashKey%2C+nsCString%3E+%3E%3A%3APutEntry%28nsACString_internal+const%26%29 https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate+|+nsTHashtable%3CnsBaseHashtableET%3CnsISupportsHashKey%2C+nsCString%3E+%3E%3A%3APutEntry%28nsISupports*%29 https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate+|+nsTHashtable%3CnsBaseHashtableET%3CnsPtrHashKey%3CnsPIDOMWindow%3E%2C+nsAutoPtr%3CnsTArray%3Cmozilla%3A%3Adom%3A%3Aworkers%3A%3AWorkerPrivate*%3E+%3E+%3E+%3E%3A%3APutEntry%28nsPIDOMWindow*%29
Comment 1•8 years ago
|
||
aklotz, there are some unexpected stacks here; PluginHangUIParent::RecvUserResponse appears to be called even though the main thread is not blocked on a plugin hang? Perhaps the PluginHangUIParent isn't disabling itself completely when the plugin hang recovers?
Updated•8 years ago
|
Flags: needinfo?(aklotz)
Assignee | ||
Comment 2•8 years ago
|
||
Adds a check to ensure that a cancellation isn't already pending when processing a user response.
Attachment #726573 -
Flags: review?(benjamin)
Flags: needinfo?(aklotz)
Updated•8 years ago
|
Attachment #726573 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Let's see if this helps. https://hg.mozilla.org/integration/mozilla-inbound/rev/1cad363e90ae
Whiteboard: [leave open]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → aklotz
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 726573 [details] [diff] [review] Potential fix [Approval Request Comment] Bug caused by (feature/regressing bug #): New feature, Plugin Hang UI (bug 805591) User impact if declined: Intermittent crashes, particularly on slower machines Testing completed (on m-c, etc.): Landed on m-c, also tested manually. Not 100% sure that this completely eliminates the crash but this is a reasonable preventative measure that won't harm anything. Risk to taking this patch (and alternatives if risky): Low, adds an additional sanity check String or UUID changes made by this patch: None
Attachment #726573 -
Flags: approval-mozilla-beta?
Attachment #726573 -
Flags: approval-mozilla-aurora?
Comment 6•8 years ago
|
||
Comment on attachment 726573 [details] [diff] [review] Potential fix We can take this on Aurora and analyze the data over the coming weeks to see if this does help with volume but it's too late to take a speculative fix on beta and this is not a topcrasher so I think we're OK to wait and let this bake.
Attachment #726573 -
Flags: approval-mozilla-beta?
Attachment #726573 -
Flags: approval-mozilla-beta-
Attachment #726573 -
Flags: approval-mozilla-aurora?
Attachment #726573 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/939d9a298677
Reporter | ||
Updated•8 years ago
|
Crash Signature: , nsCOMPtr<nsIXULTemplateBuilder> > >::PutEntry(nsISupports*) ]
[@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey, nsCOMPtr<nsISupports> > >::PutEntry(char const*) ] → , nsCOMPtr<nsIXULTemplateBuilder> > >::PutEntry(nsISupports*) ]
[@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey, nsCOMPtr<nsISupports> > >::PutEntry(char const*) ]
[@ PL_DHashTableOperate | nsTHashtable<nsURIHashKey>::PutEntry(…
Updated•8 years ago
|
Priority: -- → P2
Reporter | ||
Comment 8•8 years ago
|
||
(In reply to Aaron Klotz [:aklotz] from comment #7) > https://hg.mozilla.org/releases/mozilla-aurora/rev/939d9a298677 There are still crashes in 21.0 Beta, 22.0a2 and 23.0a1. See https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate+|+nsTHashtable%3CnsBaseHashtableET%3CnsURIHashKey%2C+nsRefPtr%3CnsXULPrototypeDocument%3E+%3E+%3E%3A%3APutEntry%28nsIURI*%29 and https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate+|+nsTHashtable%3CUnassociatedIconHashKey%3E%3A%3APutEntry%28nsIURI*%29
Crash Signature: , JSObject*> >::PutEntry(nsXBLPrototypeHandler*) ] → , JSObject*> >::PutEntry(nsXBLPrototypeHandler*) ]
[@ PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsURIHashKey, nsRefPtr<nsXULPrototypeDocument> > >::PutEntry(nsIURI*) ]
[@ PL_DHashTableOperate | nsTHashtable<UnassociatedIconHashKey>::PutEntry…
status-firefox23:
--- → affected
Assignee | ||
Comment 9•8 years ago
|
||
This crash is happening when the PCrashReporter sub-actor is torn down at the same time that the Plugin Hang UI calls TerminateChildProcess from another thread. I think what we need here is to ensure that there is mutual exclusion between destruction of the PluginModuleParent's sub-actors and PluginModuleParent::TerminateChildProcess.
Attachment #819172 -
Flags: review?(benjamin)
Comment 10•8 years ago
|
||
Comment on attachment 819172 [details] [diff] [review] Mutual exclusion fix I'm confused. What is the shared state that this lock is protecting, and why isn't the normal channel locking scheme insufficient?
Updated•8 years ago
|
Flags: needinfo?(aklotz)
Assignee | ||
Comment 11•8 years ago
|
||
The shared state is the PCrashReporterParent object. If the channel ends up closing (and thus destroying the PCrashReporterParent sub-actor) concurrently with the crash report annotations being done by the Plugin Hang UI on another thread, we end up with this crash. Unless I missed something, it doesn't look to me like the channel lock is held when OnChannelClose and OnChannelError are invoked.
Flags: needinfo?(aklotz)
Comment 12•8 years ago
|
||
Comment on attachment 819172 [details] [diff] [review] Mutual exclusion fix I'm worried about this. The locking surface is pretty huge with OnChannelClose/OnChannelError. Can we instead save CrashReporterParent* as a member (so that we don't have to lock around the IPDL data structures) and lock only in ActorDestroy? In either case, I'd like a better comment explaining that this protects both CrashReporter() method itself as well as adding annotations, and why this is safe (because plugin child processes never send annotations during normal operation, presumably).
Attachment #819172 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 13•7 years ago
|
||
This version saves the CrashReporterParent* as a member and only needs to lock in DeallocPCrashReporterParent, instead of OnChannelClose and OnChannelError.
Attachment #819172 -
Attachment is obsolete: true
Attachment #8341147 -
Flags: review?(benjamin)
Updated•7 years ago
|
Attachment #8341147 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 14•7 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=c18e5c591b1e https://hg.mozilla.org/integration/mozilla-inbound/rev/5f07f8e85831
Assignee | ||
Updated•7 years ago
|
Whiteboard: [leave open]
Comment 15•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5f07f8e85831
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment 16•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/77ac974d69d6
status-firefox25:
--- → wontfix
status-firefox26:
--- → wontfix
status-firefox27:
--- → fixed
status-firefox28:
--- → fixed
Comment 17•7 years ago
|
||
Marking verified fixed based on crash-stats.
You need to log in
before you can comment on or make changes to this bug.
Description
•