Bugzilla

Boris Zbarsky [:bzbarsky]

Comment 1

•

13 years ago

Attached patch Bandaid — Details — Splinter Review

This patch avoids the crash (and a subsequent one), but I highly doubt it's the correct way to do so.  The problem is as follows:


> NS_IMETHODIMP nsWebBrowser::InternalDestroy()
> {
>
>    if (mInternalWidget) {
>      mInternalWidget->SetClientData(0);
>      mInternalWidget->Destroy();
>      mInternalWidget = nsnull; // Force release here.
>    }
> 
>    SetDocShell(nsnull);
>
>    if(mDocShellTreeOwner)
>       {
>       mDocShellTreeOwner->WebBrowser(nsnull);
>       NS_RELEASE(mDocShellTreeOwner);
>       }

SetDocShell(nsnull) obviously clears the mDocShell member.  However, then mDocShellTreeOwner->WebBrowser(nsnull) calls DocShellTreeOwner::RemoveChromeListeners(), which then calls GetPIDOMEventTarget:


> static nsresult
> GetPIDOMEventTarget( nsWebBrowser* inBrowser, nsPIDOMEventTarget** aTarget)
> {
>   nsCOMPtr<nsIDOMWindow> domWindow;
>   inBrowser->GetContentDOMWindow(getter_AddRefs(domWindow));
>   NS_ENSURE_TRUE(domWindow, NS_ERROR_FAILURE);

The inBrowser is the same browser from earlier in the call stack, and GetContentDOMWindow tries to use the mDocShell which was previously cleared.  The method fails, so GetPIDOMEventTarget returns NULL, which we then dereference in RemoveChromeListeners().

Once this crash is worked around, we get another from the NS_RELEASE(mDocShellTreeOwner) in nsWebBrowser::InternalDestroy().  The tree owner dies, and calls RemoveChromeListeners() in its destructor.  This ends up in GetPIDOMEventTarget once more, but this time passing the cleared mWebBrowser variable, so we get another NULL dereference at |inBrowser->GetContentDOMWindow(getter_AddRefs(domWindow));|

Comment 2

•

13 years ago

We haven't made local changes to nsWebBrowser, right?  So this is just broken for all consumers right now?

We should just fix this mess to hold refs on the stack as needed.... and keep track of what objects are gone.

Tero Koskinen

Updated

•

13 years ago

Blocks: 559544

Boris Zbarsky [:bzbarsky]

Updated

•

13 years ago

Blocks: 516521

Updated

•

13 years ago

blocking2.0: --- → ?

Robert Sayre

Updated

•

13 years ago

blocking2.0: ? → final+

Oleg Romashin (:romaxa)

Updated

•

13 years ago

Attachment #442090 - Flags: review?(Olli.Pettay)

Oleg Romashin (:romaxa)

Comment 5

•

13 years ago

Any progress with this bug? any help needed? I'm using attached patch now, and it works fine.

Olli Pettay [:smaug][bugs@pettay.fi]

Updated

•

13 years ago

Attachment #442090 - Flags: review?(Olli.Pettay) → review+

Mark Finkle (:mfinkle) (use needinfo?)

Updated

•

13 years ago

tracking-fennec: --- → ?

Comment 7

•

13 years ago

This continues to be a problem for people.  Technically we could check in the patch I wrote, but it was never intended for that purpose from my point of view.  If somebody else understands the code here, they're welcome to dive in and either do a correct patch or reassure me that my changes are actually beneficial/correct.  I don't really have the time right now to do the digging myself.

Comment 8

•

13 years ago

http://hg.mozilla.org/projects/electrolysis/rev/6dc80b2a7bc0

olli, could you see comment 7. You review this patch.  I assume that means you are happy with the approach (and thus I pushed this patch to e10s).  Leaving open until I hear back from you.

Comment 10

•

13 years ago

Fwiw, this makes Electrolyis Fennec builds very hard to use.

Updated

•

13 years ago

Summary: Fennec/e10s: Content process crash on shutdown [@nsDocShellTreeOwner::RemoveChromeListeners] → [e10s] Fennec: Content process crash on shutdown [@nsDocShellTreeOwner::RemoveChromeListeners]

Comment 11

•

13 years ago

mw22 - this fix is in.  you should see it in today's nightly build.

Comment 12

•

13 years ago

I still see this crash, using:
Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:2.0b2pre) Gecko/20100630 Namoroka/4.0b2pre Fennec/2.0a1pre

I downloaded that build from:
http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-electrolysis-maemo5-gtk/fennec_2.0~a1~20100630041515_armel.deb

Am I doing something wrong?

Comment 13

•

13 years ago

When you say "this crash", do you actually mean this one?  I know of a couple shutdown problems which come and go - could I see a stack trace?

Mark Finkle (:mfinkle) (use needinfo?)

Comment 14

•

13 years ago

Sorry, I don't know how to get a stacktrace on the n900. I filed bug 576068 for the crashes that I'm seeing. I did send the crash reports to the Maemo crash reporter. Is that useful?

Comment 15

•

13 years ago

(In reply to comment #14)
> Sorry, I don't know how to get a stacktrace on the n900. I filed bug 576068 for
> the crashes that I'm seeing. I did send the crash reports to the Maemo crash
> reporter. Is that useful?

Yes. Can you link to the reports here?

Comment 16

•

13 years ago

It's not the Mozilla crash reporter that is coming up. I'm getting the Maemo crash reporter, which supposedly is sending these crashes to https://files.maemo.org/nitro/
But that site is password protected.
Should the Mozilla crash reporter have come up?

I tried again with e10s Fennec build from 2010-07-01, I'm still getting these crashes on the n900.

Comment 17

•

13 years ago

Ok, chinook builds don't crash, so I was able to type about:crashes and find out what stacktrace these crashes consist about.

This is a typical stacktrace of the Fennec crashes that I seem to be getting all the time:
http://crash-stats.mozilla.com/report/index/bp-7a9a8d77-4406-4771-aaa6-ec6222100705
0  	libxul.so  	mozilla::ipc::RPCChannel::CxxStackFrame::CxxStackFrame  	 RPCChannel.h:220
1 	libxul.so 	mozilla::ipc::RPCChannel::Send 	ipc/glue/RPCChannel.cpp:141
2 	libxul.so 	mozilla::dom::PContentProcessParent::SendSetOffline 	PContentProcessParent.cpp:254
3 	libxul.so 	mozilla::dom::ContentProcessParent::Observe 	dom/ipc/ContentProcessParent.cpp:325
4 	libxul.so 	nsObserverList::NotifyObservers 	xpcom/ds/nsObserverList.cpp:130
5 	libxul.so 	nsObserverService::NotifyObservers 	xpcom/ds/nsObserverService.cpp:182
6 	libxul.so 	nsIOService::SetOffline 	netwerk/base/src/nsIOService.cpp:696
7 	libxul.so 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:190
8 	libxul.so 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:3028
9 	libxul.so 	XPC_WN_GetterSetter 	js/src/xpconnect/src/xpcprivate.h:2558
10 	libmozjs.so 	js_Invoke 	js/src/jsinterp.cpp:654
11 	libmozjs.so 	js_InternalInvoke 	js/src/jsinterp.cpp:694
12 	libmozjs.so 	js_InternalGetOrSet 	js/src/jsinterp.cpp:730
13 	libmozjs.so 	JSScopeProperty::set 	js/src/jsscope.h:1019
14 	libmozjs.so 	js_NativeSet 	js/src/jsobj.cpp:4819
15 	libmozjs.so 	js_SetPropertyHelper 	js/src/jsobj.cpp:5222
16 	libmozjs.so 	js_Interpret 	js/src/jsops.cpp:1827
17 	libmozjs.so 	js_Invoke 	js/src/jsinterp.cpp:664
18 	libmozjs.so 	js_fun_apply 	js/src/jsfun.cpp:2062
19 	libmozjs.so 	js_Interpret 	js/src/jsops.cpp:2148
20 	libmozjs.so 	js_Invoke 	js/src/jsinterp.cpp:664
21 	libxul.so 	nsXPCWrappedJSClass::CallMethod 	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1689
22 	libxul.so 	nsXPCWrappedJS::CallMethod 	js/src/xpconnect/src/xpcwrappedjs.cpp:570
23 	libxul.so 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:132
24 	libxul.so 	libxul.so@0xe61fab

Comment 18

•

13 years ago

And the crash reporter seems to be working again in the latest builds, btw.

Comment 19

•

13 years ago

Ok, the crash you're looking at is bug 570132 which is ready to be checked in by anybody; I just haven't had the time to do so.

Comment 20

•

13 years ago

Unfortunately, the fix for bug 570132 didn't fix the crashes that I'm seeing. Apparently, those breakpad reports were unrelated.

I'm seeing those crashes with the fremantle qt trunk builds of Fennec on the n900. I haven't seen them with the chinook trunk builds. The crash reporter doesn't come up, so I can't give crash reports. Only the Maemo crash reporter appears.

Comment 21

•

13 years ago

Martijn, can you move this problem to a new bug, in that case?  CC me and anybody else who might have ideas; to my knowledge all known shutdown crash bugs are resolved at this point.

Comment 22

•

13 years ago

I was CC-ed to bug 577740. I guess I'm suffering from that bug?
In any case, the crashes that I get are happening while loading a page or shortly after that (within one minute).