Crash [@ mozilla::layout::RenderFrameParent::GetLayerManager]

RESOLVED FIXED in Firefox 9

Status

()

Core
Layout: View Rendering
--
critical
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: nhirata, Assigned: cjones)

Tracking

({crash})

8 Branch
mozilla10
ARM
Android
crash
Points:
---

Firefox Tracking Flags

(firefox9 fixed)

Details

(Whiteboard: [mobile-crash] [inbound], crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-6ebf49e2-2163-449e-89a7-a2d832110920 .
============================================================= 

Frame 	Module 	Signature [Expand] 	Source
0 	libxul.so 	mozilla::layout::RenderFrameParent::GetLayerManager 	nsIDocument.h:485
1 	libxul.so 	mozilla::layout::RenderFrameParent::AllocPLayers 	layout/ipc/RenderFrameParent.cpp:720
2 	libxul.so 	mozilla::layout::PRenderFrameParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PRenderFrameParent.cpp:105
3 	libxul.so 	mozilla::dom::PContentParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PContentParent.cpp:689
4 	libxul.so 	mozilla::ipc::AsyncChannel::OnDispatchMessage 	ipc/glue/AsyncChannel.cpp:294
5 	libxul.so 	mozilla::ipc::RPCChannel::OnMaybeDequeueOne 	ipc/glue/RPCChannel.cpp:435
6 	libxul.so 	RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run 	ipc/chromium/src/base/task.h:308
7 	libxul.so 	mozilla::ipc::RPCChannel::DequeueTask::Run 	RPCChannel.h:487
8 	libxul.so 	MessageLoop::RunTask 	ipc/chromium/src/base/message_loop.cc:343
9 	libxul.so 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/message_loop.cc:353
10 	libxul.so 	MessageLoop::DoWork 	ipc/chromium/src/base/message_loop.cc:450
11 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:115
12 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:219
13 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:511
14 	libxul.so 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:191
15 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:223
16 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3572
17 	libxul.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	toolkit/xre/nsAndroidStartup.cpp:132
18 	libmozutils.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
19 	libdvm.so 	libdvm.so@0x17037 	
20 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
21 	2 (deleted) 	2 @0x111d2f 	
22 	libdvm.so 	libdvm.so@0x171ff 	
23 	libdvm.so 	libdvm.so@0x45786 	
24 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex@0x10169 	
25 	libmozutils.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
26 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
27 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
28 	libdvm.so 	libdvm.so@0xa3447 	
29 	2 (deleted) 	2 @0x111d2f 	
30 	libmozutils.so 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
31 	libdvm.so 	libdvm.so@0x4aed0 	
32 	core.odex 	core.odex@0xe616f 	
33 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex@0x9ead 	
34 	2 (deleted) 	2 @0x111d2f 	
35 	libdvm.so 	libdvm.so@0x1bfa3 	
36 	libdvm.so 	libdvm.so@0xa3333 	
37 	libdvm.so 	libdvm.so@0xa7f4f 	
38 	libdvm.so 	libdvm.so@0x22c07 	
39 	libdvm.so 	libdvm.so@0x22b87 	
40 	libdvm.so 	libdvm.so@0x21aa3 	
41 	core.odex 	core.odex@0xe4ffb 	
42 	2 (deleted) 	2 @0x163497 	
43 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22ba7b 	
44 	libdvm.so 	libdvm.so@0x1bce7 	
45 	libdvm.so 	libdvm.so@0x1bd27 	
46 	libdvm.so 	libdvm.so@0x1bc07 	
47 	libdvm.so 	libdvm.so@0x1bc2f 	
48 	libdvm.so 	libdvm.so@0x1bc5f 	
49 	libdvm.so 	libdvm.so@0x1bc83 	
50 	libdvm.so 	libdvm.so@0x7c0cc 	
51 	core.odex 	core.odex@0x147139 	
52 	core.odex 	core.odex@0x147139 	
53 	core.odex 	core.odex@0x147187

More Crashes : https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2011-09-25&signature=mozilla%3A%3Alayout%3A%3ARenderFrameParent%3A%3AGetLayerManager&version=Fennec%3A7.0b5

See Bug 603680; seems to be the same crash?
Have you found a testcase or STR?

This looks like a null deref

LayerManager*
RenderFrameParent::GetLayerManager() const
{
  nsIDocument* doc = mFrameLoader->GetOwnerDoc();
  return doc->GetShell()->GetLayerManager();        <==== HERE
}

of null |doc|.

bz, is a document-less frame loader a state we need to check for here?
nsFrameLoader::GetOwnerDoc looks like this:

  248   { return mOwnerContent ? mOwnerContent->GetOwnerDoc() : nsnull; }

At this point GetOwnerDoc never returns null except in the middle of the document and element being cycle collected. But mOwnerContent can absolutel go away if someone is holding a strong ref to the frameloader and the element gets GCed.  Note that if that happens there will be a destroy() call on the frameloader.

Can a RenderFrameParent still be running after a destroy() on its mFrameLoader?  If so, it should probably have checks for the frameloader being already destroyed.
Yes, it looks like what could be happening is
 - nsFrameLoader::Destroy() is called, which calls TabParent::Destroy()
 - TabParent::Destroy() starts the destruction process, and calls RenderFrameParent::Destroy() for all live frames
 - RenderFrameParent::Destroy() kicks off its own destruction process, and ...

... doesn't record that the mFrameLoader was destroyed!  So it looks like if we try to create a new layer tree in the content process after nsFrameLoader::Destroy(), we could tickle this bug.  (If there's already a layer created but a paint comes in after nsFrameLoader::Destroy(), which is common, then nothing bad happens.)  That situation could arise if a tab is opened then closed really fast, but it would be a totally unreliable test.

I think I know how to patch this.
Assignee: nobody → jones.chris.g
Created attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()
Attachment #562670 - Flags: review?(bzbarsky)
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

r=me
Attachment #562670 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5456273e5ab5
Whiteboard: [mobile-crash] → [mobile-crash] [inbound]
https://hg.mozilla.org/mozilla-central/rev/5456273e5ab5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Can we have this pushed to Aurora please?  This crasher also happens in Aurora.
What's the process for landing upstream now?  Do we have to request approval somewhere?
cjones, apparantly one would "set the approval aurora flag on the patch attachment , and make a comment about why it's important.

The release team evaluates those several times a week. If you think it's valuable to advocate for it in person, you can come to one of the meetings on Tuesday and Thursday at 2pm or talk ahead of time with someone who attends those meetings. "

This was from Asa.  I don't have the rights to set an approval aurora flag on the patch attachment.  Can you do this please?
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

Requesting aurora approval.  This is a safe patch to make sure that we're not trying to operate on already-destroyed frame managers.
Attachment #562670 - Flags: approval-mozilla-aurora?
Thanks guys.

Comment 13

6 years ago
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

Approved for releases/mozilla-aurora
Attachment #562670 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Chris, you going to push this to aurora, or want me to?
Do you have anything else to ride along with it?  I should have time this evening, but I don't have an aurora clone yet.  If you're ready to fire away, please feel free! :)
http://hg.mozilla.org/releases/mozilla-aurora/rev/c9155f2b618f

Updated

6 years ago
status-firefox9: --- → fixed
You need to log in before you can comment on or make changes to this bug.