alex_mayorga Reporter
	Description • 9 years ago

This bug was filed from the Socorro interface and is 
report bp-677205bf-91da-40d7-aca1-3407d2150712.
=============================================================

https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Aa11y%3A%3ADocAccessibleParent%3A%3AUnbind%28%29

This has gone up 11 ranks ans is now top crasher 14 for 42.

It seems to be Windows only.

Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::a11y::DocAccessibleParent::Unbind() 	accessible/ipc/DocAccessibleParent.h
1 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
2 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
3 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
4 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
5 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
6 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
7 	xul.dll 	mozilla::a11y::ProxyAccessible::Shutdown() 	accessible/ipc/ProxyAccessible.cpp
8 	xul.dll 	mozilla::a11y::DocAccessibleParent::RecvHideEvent(unsigned __int64 const&) 	accessible/ipc/DocAccessibleParent.cpp
9 	xul.dll 	mozilla::a11y::PDocAccessibleParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PDocAccessibleParent.cpp
10 	xul.dll 	mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) 	obj-firefox/ipc/ipdl/PContentParent.cpp
11 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
12 	xul.dll 	mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
13 	xul.dll 	RunnableMethod<mozilla::ipc::MessageChannel, bool ( mozilla::ipc::MessageChannel::*)(void), Tuple0>::Run() 	ipc/chromium/src/base/task.h
14 	xul.dll 	MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
15 	xul.dll 	mozilla::ipc::DoWorkRunnable::Run() 	ipc/glue/MessagePump.cpp
16 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
17 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
18 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
19 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
20 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
21 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
22 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp
23 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
24 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
25 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
26 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp
27 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp
28 	firefox.exe 	NS_internal_main(int, char**) 	browser/app/nsBrowserApp.cpp
29 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp
30 	firefox.exe 	__tmainCRTStartup 	f:/dd/vctools/crt/crtw32/startup/crt0.c:255
31 	kernel32.dll 	BaseThreadInitThunk 	
32 	ntdll.dll 	RtlUserThreadStart 	
33 	kernel32.dll 	BasepReportFault 	
34 	kernel32.dll 	BasepReportFault

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 1 • 9 years ago

URLs associated with these reports all seem to be mail.yahoo.com related

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 2 • 9 years ago

Sorry not quite all, here is an example of an odd one out: http://in.bookmyshow.com/movies/minions-3d/ET00025778 (seems to show the page, the put up an in-content "Select Your City" dialog)

	Trevor Saunders (:tbsaunde)
	Comment 3 • 9 years ago

This is just a diagnostic patch, if the problem is that the document tree is inconsistant we should crash earlier, and if its something else we won't

	Pulsebot
	Comment 4 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/909610f2dfc2

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/909610f2dfc2

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 7 • 9 years ago

Looking up CheckDocTree in crash stats I only see two:
https://crash-stats.mozilla.com/report/index/a6c18852-3f3d-4c2a-8e46-e3cac2150719
https://crash-stats.mozilla.com/report/index/8a1824aa-b114-4e2c-bd89-358fe2150719

(interesting similar endings on those URLs but they are different stacks)

I don't see any Unbind stacks since build id: 20150715095506 (since the diagnostic patch landed).

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 8 • 9 years ago

OK we do have stacks: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Aa11y%3A%3ADocAccessibleParent%3A%3AUnbind%28%29#tab-reports

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 9 • 9 years ago

For these Unbind stacks it looks we fail here:
ParentDoc()->mChildDocs.RemoveElement(this);

We are deeply nested in mozilla::a11y::ProxyAccessible::Shutdown() calls.

	Trevor Saunders (:tbsaunde)
	Comment 10 • 9 years ago

So presumably MparentDoc is null.  I can't easily prove that, but it seems like a good guess.  That seems broken given the ProxyAccessible::Shutdown() calling Unbind() is for a parent.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 12 • 9 years ago

Comment on attachment 8652419 [details] [diff] [review]
make CheckDocTree check the entire document tree not just the subtree

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

Hmmm. I sort of want you to differentiate the !doc->mTopLevel case. Can you put that check in a auxiliary method and crash there directly (so crash stats is easier to understand)?

	Pulsebot
	Comment 13 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d82f63f06e7

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 9 years ago

Backed out for Marionette crashes.
https://treeherder.mozilla.org/logviewer.html#?job_id=13289529&repo=mozilla-inbound

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

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 16 • 9 years ago

Comment on attachment 8653000 [details] [diff] [review]
make CheckDocTree check the entire document tree not just the subtree r=davidb[

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

::: accessible/ipc/DocAccessibleParent.h
@@ +73,5 @@
>    virtual void ActorDestroy(ActorDestroyReason aWhy) override
>    {
> +    if (mShutdown) {
> +      return;
> +    }

Does this belong in a separate patch/bug?

	Trevor Saunders (:tbsaunde)
	Comment 17 • 9 years ago

(In reply to David Bolter [:davidb] from comment #16)
> Comment on attachment 8653000 [details] [diff] [review]
> make CheckDocTree check the entire document tree not just the subtree
> r=davidb[
> 
> Review of attachment 8653000 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/ipc/DocAccessibleParent.h
> @@ +73,5 @@
> >    virtual void ActorDestroy(ActorDestroyReason aWhy) override
> >    {
> > +    if (mShutdown) {
> > +      return;
> > +    }
> 
> Does this belong in a separate patch/bug?
noo, if you try and check a shutdown non top level doc you crash because it has no parent doc, and its not top level.  However that's ok in the special case of being shut down.

	Pulsebot
	Comment 18 • 7 years ago

Pushed by tsaunders@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18151dec259d
make CheckDocTree check the entire document tree not just the subtree r=davidb

	Carsten Book [:Tomcat]
	Comment 19 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/18151dec259d

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 20 • 7 years ago

We may need to back this out from nightly, if it caused the crash in bug 1329644.

	Carsten Book [:Tomcat]
	Comment 21 • 7 years ago

backed out in e68cbc3b5b3d from central

	Carsten Book [:Tomcat]
	Comment 22 • 7 years ago

nightlys retriggered too and backout merged also also to the integration trees

	alexander :surkov (:asurkov)
	Comment 23 • 6 years ago

seems fixed