Crash in mozilla::a11y::XULTreeAccessible::Shutdown

RESOLVED FIXED in Firefox -esr52

Status

()

defect
P2
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: philipp, Assigned: surkov)

Tracking

({crash, regression})

52 Branch
mozilla57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox-esr52 fixed, firefox53 wontfix, firefox54 wontfix, firefox55 wontfix, firefox56 fixed, firefox57 fixed)

Details

(crash signature)

Attachments

(1 attachment)

Reporter

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-7329c468-da0a-44a6-bdb8-7ee340170527.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::a11y::XULTreeAccessible::Shutdown() 	accessible/xul/XULTreeAccessible.cpp:147
1 	xul.dll 	mozilla::a11y::XULTreeAccessible::cycleCollection::Unlink(void*) 	accessible/xul/XULTreeAccessible.cpp:74
2 	xul.dll 	nsCycleCollector::CollectWhite() 	xpcom/base/nsCycleCollector.cpp:3332
3 	xul.dll 	nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) 	xpcom/base/nsCycleCollector.cpp:3674
4 	xul.dll 	nsCycleCollector_collectSlice(js::SliceBudget&, bool) 	xpcom/base/nsCycleCollector.cpp:4160
5 	xul.dll 	nsJSContext::RunCycleCollectorSlice() 	dom/base/nsJSEnvironment.cpp:1476
6 	xul.dll 	ICCTimerFired 	dom/base/nsJSEnvironment.cpp:1534
7 	xul.dll 	nsTimerImpl::Fire(int) 	xpcom/threads/nsTimerImpl.cpp:479
8 	xul.dll 	nsTimerEvent::Run() 	xpcom/threads/TimerThread.cpp:285
9 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1216
10 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:361
11 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:124
12 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:225
13 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:205
14 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:156
15 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:262
16 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp:283
17 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:4488
18 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp:4621
19 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:4712
20 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp:282
21 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
22 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
23 	kernel32.dll 	BaseThreadInitThunk 	
24 	ntdll.dll 	__RtlUserThreadStart 	
25 	ntdll.dll 	_RtlUserThreadStart

this crash signature on windows and linux is regressing in firefox 52 and subsequent builds. many user comments talk about bookmarking a page when this crash occurred:
https://crash-stats.mozilla.com/signature/?signature=mozilla%3A%3Aa11y%3A%3AXULTreeAccessible%3A%3AShutdown&date=%3E%3D2017-01-01T00%3A00%3A00.000Z#comments
Alex, any ideas?
Flags: needinfo?(surkov.alexander)
Assignee

Comment 2

2 years ago
Most likely it's a case of 2nd time shutdown. I think it may happen if js script keeps an accessible longer than it lives. This crash may be fixed by adding a simple null check for mDoc. Note, XULTreeGridRowAccessible should be fixed as well. Anyone willing to pick up this bug?
Blocks: 1133322
Flags: needinfo?(surkov.alexander)

Comment 3

2 years ago
I can reproduce the crash very easy on Nightly57.0a1 windows10 + a11y activated by Japanese IME ATOK.

Reproducible: 100%

Steps To Reproduce:
1. Open several tabs
2. Open Bookmarks sidebar and expand all tree
3. Right click on a tab and choose "Bookmarks All Tabs"
4. Click on dropdown marker "Show all bookmark folders"
5. Click on various folders. repeat several times (10-20times)
6. Click on Cancel button

Actual Results:
Browser crashes
Marco want to try this one?
Flags: needinfo?(mzehe)
Priority: -- → P2
Can attempt a fix, but since I'm no mouse user, will probably not get it to crash. I tried once when this bug was filed and couldn't reproduce it. Keeping NI as a reminder.
Assignee

Comment 6

2 years ago
Posted patch patchSplinter Review
Assignee: nobody → surkov.alexander
Attachment #8906680 - Flags: review?(mzehe)
Flags: needinfo?(mzehe)
Attachment #8906680 - Flags: review?(mzehe) → review+
https://hg.mozilla.org/mozilla-central/rev/09df59c389f5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Could you request approval for an uplift to beta please? We are shipping RC next week.
Flags: needinfo?(surkov.alexander)
Assignee

Comment 11

2 years ago
Comment on attachment 8906680 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1133322
[User impact if declined]: crashes
[Is this code covered by automated tests?]: has a fair coverage
[Has the fix been verified in Nightly?]: landed on nightly, not reliable steps to reproduce
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:no
[Is the change risky?]:low risk
[Why is the change risky/not risky?]: a null check
[String changes made/needed]:no
Flags: needinfo?(surkov.alexander)
Attachment #8906680 - Flags: approval-mozilla-beta?
Attachment #8906680 - Flags: approval-mozilla-esr52?
Comment on attachment 8906680 [details] [diff] [review]
patch

Crash fix, not new but nice to fix. let's take this for beta 12.
Attachment #8906680 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to alexander :surkov from comment #11)
> [Needs manual test from QE? If yes, steps to reproduce]: no

Marking this issue as qe-verify- based on Alexander's assessment on manual testing needs.
Flags: qe-verify-
Comment on attachment 8906680 [details] [diff] [review]
patch

null dereference fix for a11y, esr52.4+
Attachment #8906680 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.