Closed Bug 1368269 Opened 4 years ago Closed 4 years ago

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


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

52 Branch



Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- fixed
firefox57 --- fixed


(Reporter: philipp, Assigned: surkov)



(Keywords: crash, regression)

Crash Data


(1 file)

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/
13 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/
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:
Alex, any ideas?
Flags: needinfo?(surkov.alexander)
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)
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.
Attached patch patchSplinter Review
Assignee: nobody → surkov.alexander
Attachment #8906680 - Flags: review?(mzehe)
Flags: needinfo?(mzehe)
Attachment #8906680 - Flags: review?(mzehe) → review+
Closed: 4 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)
Comment on attachment 8906680 [details] [diff] [review]

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]

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]

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.