Closed Bug 543376 Opened 14 years ago Closed 14 years ago

[OOPP] Crash [@ nsPluginHost::GetPluginName(nsIPluginInstance*, char const**) ]

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: u88484, Assigned: jaas)

References

Details

Attachments

(1 file, 3 obsolete files)

I was filing my taxes on turbo tax when Minefield froze, killed mozilla-runtime.exe, everything was fine for a minute or two and then Minefield crashed.

http://crash-stats.mozilla.com/report/index/3b2aa493-e57a-42ae-9c7d-088d22100131

Signature	nsPluginHost::GetPluginName(nsIPluginInstance*, char const**)
UUID	3b2aa493-e57a-42ae-9c7d-088d22100131
Time 	2010-01-31 14:24:39.130786
Uptime	22628
Last Crash	82910 seconds before submission
Product	Firefox
Version	3.7a1pre
Build ID	20100130043817
Branch	1.9.3
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	AuthenticAMD family 15 model 104 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x10
User Comments	
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsPluginHost::GetPluginName 	modules/plugin/base/src/nsPluginHost.cpp:5057
1 	xul.dll 	nsPluginInstanceOwner::GetPluginName 	layout/generic/nsObjectFrame.cpp:397
2 	xul.dll 	nsPluginInstanceOwner::MatchPluginName 	layout/generic/nsObjectFrame.cpp:417
3 	xul.dll 	xul.dll@0xa2a7fb 	
4 	xul.dll 	nsESMEventCB::HandleEvent 	content/events/src/nsEventStateManager.cpp:3391
5 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:370
6 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:600
7 	xul.dll 	nsEventStateManager::DispatchMouseEvent 	content/events/src/nsEventStateManager.cpp:3421
8 	xul.dll 	nsEventStateManager::NotifyMouseOver 	content/events/src/nsEventStateManager.cpp:3542
9 	xul.dll 	nsEventStateManager::GenerateMouseEnterExit 	content/events/src/nsEventStateManager.cpp:3572
10 	xul.dll 	nsEventStateManager::PreHandleEvent 	content/events/src/nsEventStateManager.cpp:1127
11 	xul.dll 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:6524
12 	xul.dll 	PresShell::HandlePositionedEvent 	layout/base/nsPresShell.cpp:6380
13 	xul.dll 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:6244
14 	xul.dll 	nsViewManager::HandleEvent 	view/src/nsViewManager.cpp:1143
15 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1122
16 	xul.dll 	HandleEvent 	view/src/nsView.cpp:160
17 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:3002
18 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/src/windows/nsWindow.cpp:3025
19 	xul.dll 	nsWindow::DispatchMouseEvent 	widget/src/windows/nsWindow.cpp:3408
20 	xul.dll 	ChildWindow::DispatchMouseEvent 	widget/src/windows/nsWindow.cpp:7134
I can reproduce this every time as well on trunk.  This should be nominated to block.

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100131 Minefield/3.7a1pre.  I was on 32-bit win7 as well.

http://crash-stats.mozilla.com/report/index/bp-a76b6d1f-5237-45cc-a544-1cafa2100131

Repro:
1) open 1-2 additional plugin processes (mozilla-runtime.exe).  I opened a tab playing flash, and another with a java clock applet.
2) open task manager, and find the mozilla-runtime.exe process, then force kill
3) The plugin tab stops (flash or java one), for a few seconds, and then the entire browser crashes with the stack above.

Expected:
- Force killing the tab with mozilla-runtime should only kill that tab and process.  Not the entire firefox window

Actual:
- Entire Firefox crashes with the stack from this bug.
Blocks: OOPP
Hardware: x86_64 → All
blocking2.0: --- → alpha1
Whiteboard: [OOPPTestday]
Attached patch null check, rev. 1 (obsolete) — Splinter Review
This is a regression from bug 542971: the sequence of actions is:

* nsPluginHost::PluginCrashed
* nsNPAPIPluginInstance::Stop
* The plugin destruction guard is active, so PluginDestructionGuard::DelayDestroy postpones destruction to the event loop
* In the meantime, we receive a native event which leads to this stack
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #424797 - Flags: review?(joshmoz)
Comment on attachment 424797 [details] [diff] [review]
null check, rev. 1

TagForPlugin shouldn't fail so long as it is given a valid nsNPAPIPlugin object. IIRC there are a handful of places where we make this assumption. The best fix would be to not destroy the plugin tag until we also destroy it's nsNPAPIPlugin object. If we can't do that we need to null check all calls to TagForPlugin, and in the mean time we could just back out the original patch.
Attachment #424797 - Flags: review?(joshmoz) → review-
Hrm, I think we *are* destroying the nsNPAPIPlugin object. It's just the nsNPAPIPluginInstance that's living longer, but in a stopped state.
If we don't allow TagForPlugin to fail in the end, we should clean up it's existing usage - stop null checking where we do null check and assert in the TagForPlugin function before returning nsnull.
Blocks: 542971
I think I'm hitting this same issue over in the patch for bug 539841 -- null plugin tag for a crashed plugin.
Attached patch fix v0.9 (obsolete) — Splinter Review
This isn't done yet, just saving progress. This approach fixes the relationship between instances and their plugin. It allows instances to become safely orphaned if they are held alive by something outside of the plugin module.
Assignee: benjamin → joshmoz
Attached patch fix v0.95 (obsolete) — Splinter Review
Attachment #424876 - Attachment is obsolete: true
I backed out Bug 542971 due to this regression, per bsmedberg's request. See bug 542971 comment 6.
Attached patch fix v1.0Splinter Review
Attachment #424891 - Attachment is obsolete: true
Severity: normal → critical
I'm morphing this from a crash bug, which no longer exists, to being about improving nsNPAPIPluginInstance lifetime management.
Summary: [OOPP] Crash [@ nsPluginHost::GetPluginName(nsIPluginInstance*, char const**) ] → improve nsNPAPIPluginInstance lifetime management
Whiteboard: [OOPPTestday]
Could somebody confirm that the reason for this blocking alpha 1 is also fixed by disabling OOPP for alpha 1?
Yeah, the blocking has been resolved by the backout.
blocking2.0: alpha1 → ---
Attachment #424797 - Attachment is obsolete: true
No longer blocks: OOPP
Severity: critical → normal
Keywords: crash
OS: Windows 7 → All
I'm taking care of the instance lifetime management stuff I mentioned in comment 12 elsewhere, so sorry for changing again but I'm going back to the original summary and resolving this as fixed by the backout.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Summary: improve nsNPAPIPluginInstance lifetime management → [OOPP] Crash [@ nsPluginHost::GetPluginName(nsIPluginInstance*, char const**) ]
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: