828207 - crash in mozilla::layout::RenderFrameParent::RenderFrameParent

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Michael Vines [:m1] [:evilmachines]]

Attachment: decoded minidump of crash

Seen multiple times during stability test.  See attached for minidump.  Top frames:

Crash reason:  SIGSEGV
Crash address: 0x0

Thread 0 (crashed)
 0  libxul.so!mozilla::layout::RenderFrameParent::RenderFrameParent [RenderFrameParent.cpp : 603 + 0x0]
     r4 = 0x4233cd00    r5 = 0x00000000    r6 = 0x43c8b7e0    r7 = 0x423123c0
     r8 = 0xbe9445e8    r9 = 0x0128ebf0   r10 = 0xbe9445e4    fp = 0x417ebd48
     sp = 0xbe9444c8    lr = 0x40bbdd79    pc = 0x40b2d8d8
    Found by: given as instruction pointer in context
 1  libxul.so!mozilla::dom::TabParent::AllocPRenderFrame [TabParent.cpp : 1121 + 0x3]
     r4 = 0x4233cd00    r5 = 0xbe9445ec    r6 = 0x43c8b7e0    r7 = 0xbe9445e8
     r8 = 0xbe9445e4    r9 = 0x00000000   r10 = 0x00000001    fp = 0xbe944580
     sp = 0xbe944518    pc = 0x41075663
    Found by: call frame info
 2  libxul.so!mozilla::dom::PBrowserParent::OnMessageReceived [PBrowserParent.cpp : 1789 + 0x1d]
     r4 = 0x48308710    r5 = 0xbe944754    r6 = 0x41075629    r7 = 0x0005001b
     r8 = 0xbe9445e4    r9 = 0x00000000   r10 = 0x00000001    fp = 0xbe944580
     sp = 0xbe944540    pc = 0x410a9da5
    Found by: call frame info
 3  libxul.so!mozilla::dom::PContentParent::OnMessageReceived [PContentParent.cpp : 2410 + 0x9]
     r4 = 0x42313ca0    r5 = 0xbe944754    r6 = 0xbe94476c    r7 = 0x404390c8
     r8 = 0x41a15b18    r9 = 0x00000006   r10 = 0x00000000    fp = 0x00000000
     sp = 0xbe944628    pc = 0x410b20a5
    Found by: call frame info


.extra file didn't contain anything of interest

Comment 1

[Dietrich Ayala (:dietrich)]

Any information of which actions in the stability test were happening at the point of crash?

Comment 2

[Michael Vines [:m1] [:evilmachines]]

This instance was from the browser.py test we shared.

Comment 3

[Lucas Adamski [:ladamski]]

Blocking for 1.0 not for basecamp.  Jet, can you find an owner for this?

Comment 4

[Jet Villegas (inactive)]

Does this fail consistently at a specific test, or after several stability test runs? 

Looks like a nullptr dereference, possibly OOM. I don't see any guard code around the offending line in RenderFrameParent.cpp but I also don't see much guard code elsewhere in that code. Rather than sprinkle null-checks here, I'd prefer to make an earlier allocation infallible and OOM-abort early & cleanly.

Comment 5

[Andreas Gal :gal]

We need an owner here. Jet, except if you know the code well enough to fix this yourself, we really should assign this as soon as possible. We already lost a day here.

Comment 6

[Michael Vines [:m1] [:evilmachines]]

(In reply to Jet Villegas (:jet) from comment #4)
> Does this fail consistently at a specific test, or after several stability
> test runs? 

The latter, we have only seen this crash during multihour stability runs.

Comment 7

[Jet Villegas (inactive)]

(In reply to Lucas Adamski from comment #3)
> Blocking for 1.0 not for basecamp.  Jet, can you find an owner for this?

To Anthony for a look...

Comment 8

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

I doubt this is OOM. The new RenderFrameParent is trying to retrieve the layer manager for the document that the frame belongs to (NOT the subdocument it manages), and apparently that's returning null. It's unlikely that there is not already a layer manager there considering this is a long-lived test and this process has the radio thread so I guess this process (and its widget) are old. (I can't tell for sure which process this crash is in, though.)

There's not much data to go on here. I'll produce a debugging patch.

Comment 9

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

Attachment: logging patch to help narrow down causes of null GetLayerManager

Comment 10

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

Attachment: Wallpaper fix for this bug

I believe this patch is somewhat reasonable. The information we're failing to set here is only advisory anyway.

The main danger of this patch is that the underlying problem, whatever it is, will resurface in some even less understandable way.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 700174 [details] [diff] [review]
Wallpaper fix for this bug

Looks good.  (This would be wrong-ish for non-omtc cross-process layers, but that's not really supported atm.)

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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

Comment 13

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

I'm not sure if I should just land this logging patch. It will add debug spew to all builds --- spew which is hopefully hardly ever triggered, but who knows.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Is that code relatively hot?  If not, seems useful.  (And d'oh, forgot about that patch when I pushed your fix.)

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Going to go ahead and close this, and we can decide about landing the logging separately.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/f6573002716e

Comment 16

[Ed Morley [:emorley]]

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

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.

Comment 18

[amiller]

Does not make sense to create a regression issue.

Comment 19

[Darren]

Cannot verify, need steps to black box test this issue.