787089 - Crash [@ nsStyleContext::GetRuleNode()] with deleted this

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Bob Clary [:bc] (inactive)]

1. http://altfoto.com/2012/08/el-sucio-sucio-secreto-de-las-grandes-marcas-y-sus-sensores-defectuosos

2. Crash nsStyleContext::GetRuleNode() nsIFrame::PresContext() nsRootPresContext::RequestUpdatePluginGeometry(nsIFrame*) PresShell::DoReflow(nsIFrame*, bool) PresShell::ProcessReflowCommands(bool)

Nightly Linux 32bit and Windows 7 64bit

See bug 781279 comment 15

crash address in Windows when you don't get a Flash related crash is 0xffffffffddddddf9 while in Linux it is 0xdadadaf6

On Linux I see nsStyleContext::GetRuleNode (this=0xdadadada) at ../../dist/include/nsStyleContext.h:190

bp-e7968d4a-42bb-4df7-98ef-920df2120830

Not very reproducible in opt build but pretty much so in debug.

Comment 1

[Mats Palmgren (inactive)]

This looks like a crash in nsRootPresContext::RequestUpdatePluginGeometry
doing mUpdatePluginGeometryForFrame->PresContext() on a deleted frame.
If so, I think it's mitigated by frame-poisoning.

It could be the same underlying problem as top-crash bug 785808.

Comment 2

[Scoobidiver (away)]

On 32-bit Nightly: bp-69bfff59-550b-46bc-82d6-0cae32120902.
On 64-bit Nightly: bp-0200cf0c-f76c-4e8f-b271-0e4d82120902.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I cannot reproduce this.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Nor I, on Linux x64 or Windows 7 x64, and not on WinXP in VirtualBox.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: rip out mUpdatePluginGeometryForFrame optimization

I don't think this is worth having.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

bclary, if you can reproduce this, it would be helpful if you can determine whether the above patch fixes it. Thanks!

Comment 7

[Mats Palmgren (inactive)]

I can't reproduce in a debug or asan build.  After a lot a stress testing --
opening the URL in 30 or so tabs, reload-all-tabs, closing/reloading tabs,
all while resizing the window, I finally got one crash in a Nightly Linux64:
bp-e2ef7c12-29f6-44da-b6d9-e9d952120904

Comment 8

[Bob Clary [:bc] (inactive)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6)
> bclary, if you can reproduce this, it would be helpful if you can determine
> whether the above patch fixes it. Thanks!

I applied it to Linux Nightly and was no longer able to reproduce it there on any of the 182 urls where was originally detected. The unpatched Windows 7 builds on Nightly and Aurora and the unpatched builds on Linux Aurora were able to reproduce the original crash on a few of the urls. This crash has a very low reproducibility rate, but given the data I have I agree this patch fixes the issue at hand.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df1a1a1cfaf4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df1a1a1cfaf4

Comment 11

[Daniel Veditz [:dveditz]]

(In reply to Mats Palmgren [:mats] from comment #1)
> If so, I think it's mitigated by frame-poisoning.

The addresses in comment 0 don't look very mitigated, those are deleted memory pointers. Frame-poisoned addresses are in a different range. changing to sec-critical unless I'm missing something.

If it is sec-critical we should fix on older branches, especially if live web-content is triggering this crash which might increase the odds of discovery.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think most of these crashes are regression from bug 775965, which is only in FF17 and up. So I'll go for Aurora but not other branches at this time.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 657970 [details] [diff] [review]
rip out mUpdatePluginGeometryForFrame optimization

Review of attachment 657970 [details] [diff] [review]:
-----------------------------------------------------------------

Removes fragile optimization, so should be safe.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 657970 [details] [diff] [review]
rip out mUpdatePluginGeometryForFrame optimization

Approving for Aurora - does this mean that we should untrack/mark unaffected for 16?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Lukas Blakk [:lsblakk] from comment #14)
> Approving for Aurora - does this mean that we should untrack/mark unaffected
> for 16?

Probably, but you might want to confirm my guess that this bug doesn't happen (or happens at much lower rate) on 16.

Comment 16

[Scoobidiver (away)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> Probably, but you might want to confirm my guess that this bug doesn't
> happen (or happens at much lower rate) on 16.
This crash signature is #13 top browser crasher in 16.0b2 (partially tracked by bug 754380) but I don't know if it's related to this bug.

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13)
> Comment on attachment 657970 [details] [diff] [review]
> rip out mUpdatePluginGeometryForFrame optimization
> 
> Review of attachment 657970 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Removes fragile optimization, so should be safe.

Do we know what the regressing bug was here?

Comment 18

[Alex Keybl [:akeybl]]

I see comment 12 now. Sounds like this wouldn't help bug 754380 in that case, unless we find other related regressing bugs.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2ac849fcd6b2

Comment 20

[Alex Keybl [:akeybl]]

(In reply to Scoobidiver from comment #16)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> > Probably, but you might want to confirm my guess that this bug doesn't
> > happen (or happens at much lower rate) on 16.
> This crash signature is #13 top browser crasher in 16.0b2 (partially tracked
> by bug 754380) but I don't know if it's related to this bug.

I'm not seeing the crash signatures here, or the one listed in bug 754380, at high volume in b3. I don't believe we need to track for that version any longer. Please renominate if you disagree.

Comment 21

[Alex Keybl [:akeybl]]

Please prepare a patch for the mozilla-esr10 branch the next opportunity you get. Thanks!

Comment 22

[Al Billings [:abillings - ex-MoCo]]

Any update on an ESR patch for this? It has been weeks and this is a sec-critical.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sorry, I lost track of the request in comment #21 for some reason.

I don't think this needs to be fixed on esr10 because the crashes were primarily caused by bug 775965 which never landed on esr10.

Comment 24

[Alex Keybl [:akeybl]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23)
> Sorry, I lost track of the request in comment #21 for some reason.
> 
> I don't think this needs to be fixed on esr10 because the crashes were
> primarily caused by bug 775965 which never landed on esr10.

Thanks roc, marking as unaffected on ESR10 given that.

Comment 25

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old