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

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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@[e-mail][e-mail] 	mnt@[e-mail][e-mail]@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@[e-mail][e-mail] 	mnt@[e-mail][e-mail]@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?

Comment 1

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

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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

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

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.

Comment 4

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

Attachment: Don't ask our frame loader for its layer manager after Destroy()

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

r=me

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/5456273e5ab5

Comment 7

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/5456273e5ab5

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Can we have this pushed to Aurora please?  This crasher also happens in Aurora.

Comment 9

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

What's the process for landing upstream now?  Do we have to request approval somewhere?

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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 11

[Boris Zbarsky [:bzbarsky]]

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.

Comment 12

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

Thanks guys.

Comment 13

[LegNeato]

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

Approved for releases/mozilla-aurora

Comment 14

[Boris Zbarsky [:bzbarsky]]

Chris, you going to push this to aurora, or want me to?

Comment 15

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

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! :)

Comment 16

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

http://hg.mozilla.org/releases/mozilla-aurora/rev/c9155f2b618f