899761 - Shutdown crash due to ContentParent releasing nsFrameMessageManager after the CC is dead

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Justin Lebar (not reading bugmail)]

I get this shutdown crash occasionally when running dom/browser-element/mochitest/priority:

>Program received signal SIGSEGV, Segmentation fault.
>NS_CycleCollectorSuspect3 (n=0x1620230, cp=0x0, aRefCnt=0x1620250, aShouldDelete=0x0)
>    at ../../../src/xpcom/base/nsCycleCollector.cpp:3240
>3240	    MOZ_ASSERT(data);
>(gdb) bt
>#0  NS_CycleCollectorSuspect3 (n=0x1620230, cp=0x0, aRefCnt=0x1620250, aShouldDelete=0x0)
>    at ../../../src/xpcom/base/nsCycleCollector.cpp:3240
>#1  0x00002aaaacca810f in nsCycleCollectingAutoRefCnt::decr (this=0x1620250, owner=0x1620230, p=0x0, 
>    shouldDelete=0x0) at ../../../dist/include/nsISupportsImpl.h:147
>#2  0x00002aaaacccf875 in nsCycleCollectingAutoRefCnt::decr (this=0x1620250, owner=0x1620230, 
>    shouldDelete=0x0) at ../../../dist/include/nsISupportsImpl.h:135
>#3  0x00002aaaad8e3157 in nsFrameMessageManager::Release (this=0x1620230)
>    at ../../../../src/content/base/src/nsFrameMessageManager.cpp:109
>#4  0x00002aaaad8de47d in nsRefPtr<nsFrameMessageManager>::~nsRefPtr (this=0x2c2dc48)
>    at ../../../dist/include/nsAutoPtr.h:880
>#5  0x00002aaaad8cfd45 in nsRefPtr<nsFrameMessageManager>::~nsRefPtr (this=0x2c2dc48)
>    at ../../../dist/include/nsAutoPtr.h:878
>#6  0x00002aaaaef4e91a in mozilla::dom::ContentParent::~ContentParent (this=0x2c2d820)
>    at ../../../src/dom/ipc/ContentParent.cpp:1259
>#7  0x00002aaaaef4e5b9 in mozilla::dom::ContentParent::~ContentParent (this=0x2c2d820)
>    at ../../../src/dom/ipc/ContentParent.cpp:1236
>#8  0x00002aaaaef504fe in mozilla::dom::ContentParent::Release (this=0x2c2d820)
>    at ../../../src/dom/ipc/ContentParent.cpp:1573
>#9  0x00002aaaaef57229 in RunnableMethodTraits<mozilla::dom::ContentParent>::ReleaseCallee (obj=0x2c2d820)
>    at ../../../src/ipc/chromium/src/base/task.h:263
>#10 0x00002aaaaef571ef in RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(bool), Tuple1<bool> >::ReleaseCallee (this=0x2f96c30) at ../../../src/ipc/chromium/src/base/task.h:317
>#11 0x00002aaaaef57309 in RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(bool), Tuple1<bool> >::~RunnableMethod (this=0x2f96c30) at ../../../src/ipc/chromium/src/base/task.h:302
>#12 0x00002aaaaef57105 in RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(bool), Tuple1<bool> >::~RunnableMethod (this=0x2f96c30) at ../../../src/ipc/chromium/src/base/task.h:301
>#13 0x00002aaaaef57129 in RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(bool), Tuple1<bool> >::~RunnableMethod (this=0x2f96c30) at ../../../src/ipc/chromium/src/base/task.h:301
>#14 0x00002aaaaf9dc5f5 in MessageLoop::DeletePendingTasks (this=0x501250)
>    at ../../../src/ipc/chromium/src/base/message_loop.cc:420
>#15 0x00002aaaaf9dc174 in MessageLoop::~MessageLoop (this=0x501250)
>    at ../../../src/ipc/chromium/src/base/message_loop.cc:162
>#16 0x00002aaaaf8c2765 in MessageLoopForUI::~MessageLoopForUI (this=0x501250)
>    at ../../../src/ipc/chromium/src/base/message_loop.h:436
>#17 0x00002aaaaf8c2715 in MessageLoopForUI::~MessageLoopForUI (this=0x501250)
>    at ../../../src/ipc/chromium/src/base/message_loop.h:436
>#18 0x00002aaaaf8c2739 in MessageLoopForUI::~MessageLoopForUI (this=0x501250)
>    at ../../../src/ipc/chromium/src/base/message_loop.h:436
>#19 0x00002aaaaf8c1f18 in mozilla::ShutdownXPCOM (servMgr=0x0)
>    at ../../../src/xpcom/build/nsXPComInit.cpp:803

This may be simple to fix.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1: Fix a debug-build shutdown crash in ContentParent.

Sometimes a ContentParent object gets destroyed after XPCOM shutdown has
shut down the cycle collector.  If the ContentParent destructor releases
a cycle collected object (e.g. mMessageManager), this causes the cycle
collector to assert.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 783359 [details] [diff] [review]
Patch, v1: Fix a debug-build shutdown crash in ContentParent.

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

::: dom/ipc/ContentParent.cpp
@@ +850,5 @@
>    // returned from Get*() while in the midst of shutdown.
>    MarkAsDead();
> +
> +  // A ContentParent object might not get freed until after XPCOM shutdown has
> +  // shut down the cycle colllector.  But by then it's too late to release any

only two 'l's in collector :-P

Comment 3

[Justin Lebar (not reading bugmail)]

Thanks!

https://hg.mozilla.org/projects/birch/rev/8e5568181a8a

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8e5568181a8a

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Why isn't the patch calling Disconnect on mMessageManager.