724355 - Crash nsPluginFrame::SetInstanceOwner

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Scoobidiver (away)]

It's a new crash signature that first appeared in 13.0a1/20120204.
The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e777c939a3f9&tochange=766a59650976
It's likely a regression from bug 683059.

Signature 	nsObjectFrame::SetInstanceOwner(nsPluginInstanceOwner*) More Reports Search
UUID	033d6628-f36a-423c-8603-2e4922120205
Date Processed	2012-02-05 07:50:48
Uptime	1050
Last Crash	23.0 hours before submission
Install Age	14.8 hours since version was first installed.
Install Time	2012-02-04 17:03:28
Product	Firefox
Version	13.0a1
Build ID	20120204031137
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7600
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x2fe8254a
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x9480, AdapterSubsysID: 04561028, AdapterDriverVersion: 8.783.3.0
D2D? D2D+
DWrite? DWrite+
D3D10 Layers? D3D10 Layers+
EMCheckCompatibility	True

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsObjectFrame::SetInstanceOwner 	layout/generic/nsObjectFrame.cpp:787
1 	xul.dll 	nsPluginInstanceOwner::Destroy 	dom/plugins/base/nsPluginInstanceOwner.cpp:2734
2 	xul.dll 	nsObjectLoadingContent::DoStopPlugin 	content/base/src/nsObjectLoadingContent.cpp:2036
3 	xul.dll 	nsObjectLoadingContent::StopPluginInstance 	content/base/src/nsObjectLoadingContent.cpp:2063
4 	xul.dll 	InDocCheckEvent::Run 	content/base/src/nsObjectLoadingContent.cpp:171
5 	xul.dll 	nsBaseAppShell::OnProcessNextEvent 	widget/xpwidgets/nsBaseAppShell.cpp:340
6 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:619
7 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:347
8 	xul.dll 	TimerThread::RemoveTimer 	xpcom/threads/TimerThread.cpp:435
9 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
10 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:201
11 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:175
12 	xul.dll 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:189
13 	xul.dll 	nsAppShell::Run 	widget/windows/nsAppShell.cpp:258
14 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:220
15 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3537
16 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:107

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsObjectFrame%3A%3ASetInstanceOwner%28nsPluginInstanceOwner*%29

Comment 1

[Jim Mathies [:jimm]]

This was discussed in the original bug. These are InDocCheckEvent::Run()'s firing off after nsObjectFrame's are gone. Once that problem gets tracked down and fixed these should go away.

Comment 2

[Scoobidiver (away)]

Crash reports contain jvm.dll (Java HotSpot(TM) Client VM) and it might be related to bug 285982 and bug 724248.

Comment 4

[Scoobidiver (away)]

It's #6 top crasher in 13.0a1 over the last 3 days.

Comment 5

[Scoobidiver (away)]

One comment says:
"I was running the nVidia "About Your GPU" Java applet. Around 5 minutes ago, I ran the same applet and Firefox "Stopped Responding" for around 10 seconds."

Comment 6

[Sheila Mooney]

These have gone way down in volume. I see only 4 crashes in the past 7 days. A few weeks ago we have 100 crashes. Maybe some 3rd party issue that's been fixed? I say we watch it for another week then remove the tracking flag.

Comment 7

[Josh Aas]

The patch in bug 723523 probably improved the situation.

Comment 8

[Scoobidiver (away)]

It's no longer a top crasher for both signatures.

Comment 9

[Alex Keybl [:akeybl]]

(In reply to Scoobidiver from comment #8)
> It's no longer a top crasher for both signatures.

Given that, untracking for release.

Comment 10

[Alice0775 White]

Crash with bp-176c4339-affe-4583-9274-5ceec2120601
http://hg.mozilla.org/releases/mozilla-aurora/rev/91eca10ae9a5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120531 Firefox/14.0a2 ID:20120531042008

Comment 11

[Benjamin Smedberg]

The crashes at nsPresContext::GetRootPresContext showed up in the e10s experiment as an e10s-only crash signature. These crashes are at poison + offset 0xe5e5e5ed so if this is the same bug it is a UAF and should block e10s.

Marking s-s for now.

Comment 12

[Benjamin Smedberg]

Link to the recent crashes: https://crash-stats.mozilla.com/search/?signature=~nsPresContext%3A%3AGetRootPresContext&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=address&_columns=reason#crash-reports

Comment 13

[Jim Mathies [:jimm]]

jet can you help find an owner here? Investigating this is currently blocking e10s rollout.

Comment 14

[Jet Villegas (inactive)]

Sure, I can help. Is the original signature still relevant?
nsObjectFrame::SetInstanceOwner(nsPluginInstanceOwner*)

Or, are we only interested in hunting down ::GetRootPresContext()?

Comment 15

[Jet Villegas (inactive)]

I wonder if this is the same crash bug as 1241217 moving up the stack now that bug 1241651 is fixed. tn: any thoughts on that one?

Comment 16

[Jim Mathies [:jimm]]

(In reply to Jet Villegas (:jet) from comment #14)
> Sure, I can help. Is the original signature still relevant?
> nsObjectFrame::SetInstanceOwner(nsPluginInstanceOwner*)
> 
> Or, are we only interested in hunting down ::GetRootPresContext()?

The GetRootPresContext stack. It's currently showing up as #35 in our experiment crash list - 

https://crash-stats.mozilla.org/search/?ActiveExperiment=e10s-beta46-noapz%40experiments.mozilla.org&ActiveExperimentBranch=experiment-no-addons&_facets=signature&_columns=signature&_columns=product&_columns=build_id&_columns=platform#facet-signature

Comment 17

[Timothy Nikkel (:tnikkel)]

(In reply to Jet Villegas (:jet) from comment #15)
> I wonder if this is the same crash bug as 1241217 moving up the stack now
> that bug 1241651 is fixed. tn: any thoughts on that one?

Oh dang, looks like you're right. We didn't fix anything there. Which means I have even less clue of what's going on in this bug :(

This looks like a tough one. The only road I see is code reading, random guesses (and I don't have many of those even), landing things that will provide clues in crash reports, waiting for crash reports, re-evaluating, and repeat. And with no current crashes on nightly, and only one crash on aurora that looks like a very long drawn out process.

Maybe someone else has a better idea?

Comment 18

[Daniel Veditz [:dveditz]]

Should have filed a new bug for this rather than hijack one from four years ago--there doesn't seem to be anything plugin related in these new stacks.

Comment 19

[Timothy Nikkel (:tnikkel)]

Patch in bug 1260531 might help with this.

Comment 20

[Johnny Stenback (:jst)]

Seems to me we should indeed file a new bug for the e10s crashes we're seeing and focus on that one (also security sensitive, but may or may not have anything to do with plugin frames as this bug does).

Jet, can you please file a new bug, move the e10s flags over to the new one, and make sure that bug gets the attention it deserves (blocks e10s shipping, so needs someone on it).

Comment 21

[Jet Villegas (inactive)]

OK, filed bug 1261175 for crashes related to ::GetRootPresContext()

Comment 22

[Gabor Krizsanits (INACTIVE)]

(In reply to Jet Villegas (:jet) from comment #21)
> OK, filed bug 1261175 for crashes related to ::GetRootPresContext()

Could you please also CC e10s folks to that bug who were CCed to this bug and now does not have access to the new one?

Comment 23

[Timothy Nikkel (:tnikkel)]

ni transferred to bug 1261175.

Comment 24

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Jet, it's not clear if we've covered everything in the new bug 1261175, or if there is still an issue remaining here. CritSmash group is trying to determine if this bug should stay open as a sec-critical bug - with some additional action required - or if it it should be closed out in order to continue discussion in the new bug.

Do you have any thoughts?

Comment 25

[Jet Villegas (inactive)]

I'd close this one out unless you're seeing new crashes with the original signature
nsObjectFrame::SetInstanceOwner(nsPluginInstanceOwner*)

Comment 26

[Andrew McCreight [:mccr8]]

I don't see any crashes with that signature.

Comment 27

[Mats Palmgren (inactive)]

I think the signature just changed from nsObjectFrame to nsPluginFrame.
bp-d03ffdb7-9c76-4f64-8258-3878b2160426

Comment 28

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Jet, looks like this signature changed and is still showing up in 46.0.1 and 47 beta. It may still affect 48 and 49 even though we aren't seeing crash reports. 

This and bug 1261175 (also sec-critical) still need owners.

Comment 29

[Jet Villegas (inactive)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #28)
> Jet, looks like this signature changed and is still showing up in 46.0.1 and
> 47 beta. It may still affect 48 and 49 even though we aren't seeing crash
> reports. 
> 
> This and bug 1261175 (also sec-critical) still need owners.

Matt Woodrow is looking through the raw memory dumps for more clues here (and bug 1261175.)

Comment 30

[Matt Woodrow (:mattwoodrow)]

(In reply to Mats Palmgren (:mats) from comment #27)
> I think the signature just changed from nsObjectFrame to nsPluginFrame.
> bp-d03ffdb7-9c76-4f64-8258-3878b2160426

This crash is weird.

We manage to call mWidget->Show(false) and mWidget->Enable(false), but then we mWidget is the poison value when we try call mWidget->SetParent(nullptr);

The only possibility I can think of is that |this| has already been destroyed, and jemalloc hadn't written the poison value until this point.

That would suggest that nsPluginInstanceOwner::mPluginFrame is dangling, but I can't see how this would happen.

Comment 31

[Andrew McCreight [:mccr8]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #28)
> Jet, looks like this signature changed and is still showing up in 46.0.1 and
> 47 beta. It may still affect 48 and 49 even though we aren't seeing crash
> reports. 

Now I see crashes on 48 but not 49 or 50, so it must be tied to the user population somehow.

Comment 32

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

This is still affecting 49, 50, and 45esr:  https://crash-stats.mozilla.com/signature/?signature=nsPresContext%3A%3AGetRootPresContext#bugzilla

Matt, is this still relevant or should those crashes be part of some new bug?

Comment 33

[Matt Woodrow (:mattwoodrow)]

I think this is still relevant, we just don't have any leads right now.

Comment 34

[Matt Woodrow (:mattwoodrow)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #32)
> This is still affecting 49, 50, and 45esr: 
> https://crash-stats.mozilla.com/signature/
> ?signature=nsPresContext%3A%3AGetRootPresContext#bugzilla
> 
> Matt, is this still relevant or should those crashes be part of some new bug?

Actually, those crash reports are bug 1261175, not this one.

Comment 35

[BugBot [:suhaib / :marco/ :calixte]]

Crash volume for signature 'nsPresContext::GetRootPresContext':
 - nightly (version 52): 0 crashes from 2016-09-19.
 - aurora  (version 51): 12 crashes from 2016-09-19.
 - beta    (version 50): 417 crashes from 2016-09-20.
 - release (version 49): 9 crashes from 2016-09-05.
 - esr     (version 45): 109 crashes from 2016-07-25.

Crash volume on the last weeks (Week N is from 10-17 to 10-23):
            W. N-1  W. N-2  W. N-3  W. N-4
 - nightly       0       0       0       0
 - aurora        2       0       1       1
 - beta        123     113     107      19
 - release       2       3       3       0
 - esr          13      12      17      11

Affected platforms: Windows, Mac OS X, Linux

Crash rank on the last 7 days:
           Browser   Content     Plugin
 - nightly
 - aurora            #91
 - beta    #827      #16
 - release #10010
 - esr     #590

Comment 36

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

we are a week away from RC, no fix in sight, this is now a wontfix for 50.

Comment 37

[Marco Castelluccio [:marco]]

For "nsPresContext::GetRootPresContext" on Beta, we have:
(100.0% in signature vs 28.64% overall) Module "winspool.drv" = true
(86.02% in signature vs 43.62% overall) dom_ipc_enabled = 1

So it looks like we're crashing when printing (or after) printing something (because "winspool.drv" is loaded) and we're more often crashing with e10s enabled.

Comment 38

[Frederik Braun [:freddy]]

Marco, thank you for taking a look at this. Can you take this bug?
If you can not, can you help us find someone to make sure this doesn'st stay in limbo?

Comment 39

[Marco Castelluccio [:marco]]

Unfortunately I'm not sure who can take the bug.

Comment 40

[Frederik Braun [:freddy]]

Benjamin, you are the triage owner. Please find someone to work on this sec-high bug.

Comment 41

[Andrew McCreight [:mccr8]]

(In reply to Frederik Braun [:freddyb] from comment #40)
> Benjamin, you are the triage owner. Please find someone to work on this
> sec-high bug.

Comment 42

[Benjamin Smedberg]

There are a total of 28 crashes in the past two weeks, and no steps to reproduce. Comment 37 is irrelevant because it's about the nsPresContext signature which is not part of this bug. And the signatures for this bug do not appear at all in e10s.

https://crash-stats.mozilla.com/search/?signature=%3DnsPluginFrame%3A%3ASetInstanceOwner&signature=%3DnsObjectFrame%3A%3ASetInstanceOwner&date=%3E%3D2017-01-18T16%3A42%3A02.000Z&date=%3C2017-02-01T16%3A42%3A02.000Z&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=address&_columns=reason&_columns=e10s_cohort&_columns=process_type#crash-reports

So no, I don't think there's any engineering action that can be taken on this bug.

Comment 43

[Andrew McCreight [:mccr8]]

Thanks for taking a look. Let's just close this incomplete, then.