Firefox 8.0.1 Crash Report [@ nsLinkableAccessible::GetValue(nsAString_internal&) ] (mainly correlated to WebRoot Secure Anywhere)

RESOLVED FIXED in Firefox 12

Status

()

Core
Disability Access APIs
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Tomcat, Assigned: surkov)

Tracking

({crash, topcrash})

8 Branch
mozilla14
x86
Windows 7
crash, topcrash
Points:
---

Firefox Tracking Flags

(firefox11 affected, firefox12 verified, firefox13 verified)

Details

(Whiteboard: [qa+], crash signature)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
noticed this crash on the top changer report for Firefox 8.0.1. Crash  [@ nsLinkableAccessible::GetValue(nsAString_internal&) ] - example crash report -> https://crash-stats.mozilla.com/report/index/d0216e0f-4bd2-449e-8b1a-317032111201 and general overview -> https://crash-stats.mozilla.com/report/list?range_value=3&range_unit=days&signature=nsLinkableAccessible%3A%3AGetValue%28nsAString_internal%26%29&version=Firefox%3A8.0.1

seems its there since at least Firefox 7.0.1

stack:

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsLinkableAccessible::GetValue 	accessible/src/base/nsBaseWidgetAccessible.cpp:137
1 	xul.dll 	nsAccessibleWrap::get_accValue 	accessible/src/msaa/nsAccessibleWrap.cpp:322
2 	rpcrt4.dll 	Invoke 	
3 	rpcrt4.dll 	NdrStubCall2 	
4 	ole32.dll 	NdrpCreateStub 	
5 	oleaut32.dll 	oleaut32.dll@0xffd2 	
6 	ole32.dll 	SyncStubInvoke 	
7 	ole32.dll 	StubInvoke 	
8 	ole32.dll 	CCtxComChnl::ContextInvoke 	
9 	ole32.dll 	MTAInvoke 	
10 	ole32.dll 	STAInvoke 	
11 	ole32.dll 	AppInvoke 	
12 	ole32.dll 	ComInvokeWithLockAndIPID 	
13 	ole32.dll 	ComInvoke 	
14 	ole32.dll 	ThreadDispatch 	
15 	ole32.dll 	ThreadWndProc 	
16 	user32.dll 	InternalCallWinProc 	
17 	user32.dll 	UserCallWinProcCheckWow 	
18 	user32.dll 	DispatchMessageWorker 	
19 	user32.dll 	DispatchMessageW 	
20 	xul.dll 	nsAppShell::ProcessNextNativeEvent 	widget/src/windows/nsAppShell.cpp:346
21 	xul.dll 	nsBaseAppShell::OnProcessNextEvent 	widget/src/xpwidgets/nsBaseAppShell.cpp:324
22 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:595
23 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:134
24 	xul.dll 	xul.dll@0xba37ab 	
25 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:205
26 	xul.dll 	xul.dll@0x30056f 	
27 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:179
28 	xul.dll 	mozilla::storage::AsyncExecuteStatements::AsyncExecuteStatements 	storage/src/mozStorageAsyncStatementExecution.cpp:239
29 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:189
30 	dui70.dll 	DirectUI::ValueProvider::ValueProvider 	
31 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:224
32 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3544
33 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:107
34 	firefox.exe 	firefox.exe@0x4043 	
35 	firefox.exe 	_RTC_Initialize 	
36 	mozcrt19.dll 	_initterm 	obj-firefox/memory/jemalloc/crtsrc/crt0dat.c:852
37 	firefox.exe 	__security_init_cookie 	obj-firefox/memory/jemalloc/crtsrc/gs_support.c:139
38 	kernel32.dll 	BaseThreadInitThunk 	
39 	ntdll.dll 	WinSqmSetIfMaxDWORD 	
40 	ntdll.dll 	_RtlUserThreadStart 	
41 	firefox.exe 	firefox.exe@0x1cef 	
42 	firefox.exe 	firefox.exe@0x1cef

Updated

6 years ago
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
(Assignee)

Comment 1

6 years ago
Created attachment 578219 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #578219 - Flags: review?(marco.zehe)

Comment 2

6 years ago
Comment on attachment 578219 [details] [diff] [review]
patch

r=me thanks for the quick fix!
Attachment #578219 - Flags: review?(marco.zehe) → review+
https://hg.mozilla.org/mozilla-central/rev/768c2a7a320e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Comment 4

6 years ago
There are still crashes with 64-bit builds:
https://crash-stats.mozilla.com/report/list?version=Firefox:11.0a1&range_value=30&range_unit=days&signature=nsLinkableAccessible%3A%3AGetValue%28nsAString_internal%26%29
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Trevor, I was looking at the fix over on bug 653584. It seems that nsLinkableAccessible::BindToParent can exit without assigning mActionAcc (leaving it null).

Should we be null checking in GetValue,DoAction,GetKeyboardShortcut,GetAnchorURI?

Also, I didn't dive too deep but is there any danger mActionAcc could equal /this/? (GetValue could infinitely recurse)?
OK as per IRC I see mIsLink should only be true when we have an mActionAcc. Smells a bit funny though.

I have no idea why these crashes are only happening on amd64.
Users are still getting crashes.

Comment 8

6 years ago
It's #47 top browser crasher in 9.0.1 and #46 in 10.0b2.
After the patch landing, it's #82 top browser crasher in 11.0a2 and #288 in 12.0a1.

Comment 9

5 years ago
There's a high correlation in 9.0.1 (before the patch) with WebRoot Secure Anywhere:
94% (240/255) vs.   1% (555/101230) WRusr.dll
Summary: Firefox 8.0.1 Crash Report [@ nsLinkableAccessible::GetValue(nsAString_internal&) ] → Firefox 8.0.1 Crash Report [@ nsLinkableAccessible::GetValue(nsAString_internal&) ] (mainly correlated to WebRoot Secure Anywhere)
This is showing up in Firefox 10 crash data as the #32 overall crash. Adding Kev to see if he has a contact at that company - http://www.webroot.com/En_US/consumer-products-secureanywhere-complete.html.

Comment 11

5 years ago
Before the patch, it's #35 top crasher in 10.0.
After the patch, it's #87 top crasher in 11.0b1, #61 in 12.0a2, and #146 in 13.0a1.

There are still correlations to WebRoot SecureAnywhere in 11.0:
nsLinkableAccessible::GetValue(nsAString_internal&)|EXCEPTION_ACCESS_VIOLATION_READ (30 crashes)
     90% (27/30) vs.   0% (69/26008) WRusr.dll
          0% (0/30) vs.   0% (2/26008) 8.0.1.42
         90% (27/30) vs.   0% (67/26008) 8.0.1.95

Comment 12

5 years ago
Marcia, did we try and reproduce this?

Comment 13

5 years ago
It's #18 top browser crasher in 11.0, #24 in 12.0b1.

It still happens with the latest WebRoot SecureAnywhere version:
     95% (20/21) vs.   0% (39/13377) WRusr.dll
          0% (0/21) vs.   0% (1/13377) 8.0.1.40
         95% (20/21) vs.   0% (38/13377) 8.0.1.154
Keywords: topcrash
Alexander, any ideas?
(In reply to David Bolter [:davidb] from comment #14)
> Alexander, any ideas?

I still can't think of anything, but I'd be interested to see if msvc can get us a stack trace with a little more info, such as local vars or exactly which instruction we crash on and the local disassembly
(Assignee)

Comment 16

5 years ago
a crash link pls?
(In reply to alexander :surkov from comment #16)
> a crash link pls?

http://bit.ly/GCu1gq
(Assignee)

Comment 18

5 years ago
Created attachment 607832 [details] [diff] [review]
patch: second round
Attachment #607832 - Flags: review?(trev.saunders)

Updated

5 years ago
status-firefox11: --- → affected
status-firefox12: --- → affected
status-firefox13: --- → affected
Comment on attachment 607832 [details] [diff] [review]
patch: second round

>+nsLinkableAccessible::UnbindFromParent()
>+{
>+  mActionAcc = nsnull;
>+  mIsLink = false;
>+  mIsOnclick = nsnull;

false?
Attachment #607832 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 20

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #19)

> >+  mIsOnclick = nsnull;
> 
> false?

sure, I noticed that after I filed the patch :)
(Assignee)

Comment 21

5 years ago
(In reply to alexander :surkov from comment #18)
> Created attachment 607832 [details] [diff] [review]
> patch: second round

https://hg.mozilla.org/integration/mozilla-inbound/rev/21fabfdbeaab
https://hg.mozilla.org/mozilla-central/rev/21fabfdbeaab
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla11 → mozilla14

Comment 23

5 years ago
It's #25 top browser crasher in 12.0b1 and 13.0a2.
That qualifies it for Aurora and Beta approvals.
(Assignee)

Comment 24

5 years ago
(In reply to Scoobidiver from comment #23)
> It's #25 top browser crasher in 12.0b1 and 13.0a2.
> That qualifies it for Aurora and Beta approvals.

this patch is a reasonable guess about the crash cause. What is the best way to proceed, should we get some results from Firefox 14 before nomination for back porting or should we proceed immediately?

Comment 25

5 years ago
(In reply to alexander :surkov from comment #24)
> should we get some results from Firefox 14 before nomination for
> back porting or should we proceed immediately?
Before back porting it, let's monitor it in the trunk during a few days with this link:
https://crash-stats.mozilla.com/report/list?query_search=signature&version=Firefox:14.0a1&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=nsLinkableAccessible%3A%3AGetValue%28nsAString_internal%26%29
(Assignee)

Comment 26

5 years ago
it sounds like it improves things: two crashes for 25 and 28 March builds. Makes sense to back port this patch.
Comment on attachment 607832 [details] [diff] [review]
patch: second round

[Approval Request Comment]
Regression caused by (bug #): unknown
User impact if declined: frequent crashes
Testing completed (on m-c, etc.): Yes, see comment #26
Risk to taking this patch (and alternatives if risky):None, just nulling out some members.
String changes made by this patch:None
Attachment #607832 - Flags: approval-mozilla-beta?
Attachment #607832 - Flags: approval-mozilla-aurora?
Comment on attachment 607832 [details] [diff] [review]
patch: second round

Discussed and approved in triage, land it soon please!
Attachment #607832 - Flags: approval-mozilla-beta?
Attachment #607832 - Flags: approval-mozilla-beta+
Attachment #607832 - Flags: approval-mozilla-aurora?
Attachment #607832 - Flags: approval-mozilla-aurora+
Landed on Alexander's behalf:
http://hg.mozilla.org/releases/mozilla-aurora/rev/fdce25e870da
http://hg.mozilla.org/releases/mozilla-beta/rev/833d79ffbf39
status-firefox12: affected → fixed
status-firefox13: affected → fixed

Comment 30

5 years ago
(In reply to alexander :surkov from comment #20)
> (In reply to Trevor Saunders (:tbsaunde) from comment #19)
> > >+  mIsOnclick = nsnull;
> > 
> > false?
> 
> sure, I noticed that after I filed the patch :)

I guess it doesn't matter, but the push to aurora and beta doesn't seem to have this change from nsnull to false, which was made between attaching the patch and pushing to trunk.
Alex, did you make a change before pushing to trunk? If so, please make these changes to the aurora and beta source files and push as bustage fixes.
(Assignee)

Comment 32

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #31)
> Alex, did you make a change before pushing to trunk?

yes, sometimes I do that. So the best way is to import path from trunk and then back port it to aurora or beta. To be on safe side.

> If so, please make
> these changes to the aurora and beta source files and push as bustage fixes.

I don't think it's big deal. nsnull is casted to false.
Verify fix by checking crashstats after the March 29th landing.
Whiteboard: [qa+]
http://bit.ly/HPSf4c

Since the fix landed on 12 beta, a lot less crashes than F11, but still a few:
12 crashes on 12b4
5 crashes on 12b5

Are these expected? Would it be enough to call it verified?

Comment 35

5 years ago
(In reply to Virgil Dicu [:virgil] [QA] from comment #34)
> Are these expected? Would it be enough to call it verified?

I would think so. From the experience I have dealing with crash stats every day, I'd guess the main issue we saw here has been fixed and there is a different issue just merely triggering something with the same signature. For this other issue, a different bug should be filed if it's of high enough frequency (which your numbers don't seem to imply).
Thank you for the clarification, Robert. Marking this as verified on Firefox 12 based on crash-stats data.
status-firefox12: fixed → verified
http://bit.ly/HPSf4c

Only one crash in F13beta after 2 weeks after merge. Marking as verified in F13 as well, as the main issue is resolved.
status-firefox13: fixed → verified
You need to log in before you can comment on or make changes to this bug.