Closed Bug 1184217 Opened 4 years ago Closed 2 years ago

crash in mozilla::a11y::DocAccessibleParent::Unbind()

Categories

(Core :: Disability Access APIs, defect, critical)

42 Branch
Unspecified
Windows 8.1
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: alex_mayorga, Unassigned)

References

Details

(Keywords: crash, leave-open)

Crash Data

Attachments

(3 files)

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
URLs associated with these reports all seem to be mail.yahoo.com related
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)
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
Attachment #8634384 - Flags: review?(dbolter)
Attachment #8634384 - Flags: review?(dbolter) → review+
Duplicate of this bug: 1183293
https://hg.mozilla.org/mozilla-central/rev/909610f2dfc2
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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).
For these Unbind stacks it looks we fail here:
ParentDoc()->mChildDocs.RemoveElement(this);

We are deeply nested in mozilla::a11y::ProxyAccessible::Shutdown() calls.
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.
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)?
Attachment #8652419 - Flags: review?(dbolter) → review+
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?
(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.
Attachment #8653000 - Flags: review?(dbolter) → review+
Crash Signature: [@ mozilla::a11y::DocAccessibleParent::Unbind()] → [@ mozilla::a11y::DocAccessibleParent::Unbind()] [@ mozilla::a11y::DocAccessibleParent::Unbind]
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
We may need to back this out from nightly, if it caused the crash in bug 1329644.
Flags: needinfo?(tbsaunde+mozbugs)
backed out in e68cbc3b5b3d from central
nightlys retriggered too and backout merged also also to the integration trees
Depends on: 1329644
Flags: needinfo?(tbsaunde+mozbugs)
Assignee: tbsaunde+mozbugs → nobody
seems fixed
Status: REOPENED → RESOLVED
Closed: 4 years ago2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.