Last Comment Bug 706784 - Firefox 8.0.1 Crash Report [@ nsLinkableAccessible::GetValue(nsAString_internal&) ] (mainly correlated to WebRoot Secure Anywhere)
: Firefox 8.0.1 Crash Report [@ nsLinkableAccessible::GetValue(nsAString_intern...
Status: RESOLVED FIXED
[qa+]
: crash, topcrash
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: 8 Branch
: x86 Windows 7
: -- critical (vote)
: mozilla14
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-01 02:54 PST by Carsten Book [:Tomcat]
Modified: 2012-05-09 08:01 PDT (History)
14 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
verified
verified


Attachments
patch (1.72 KB, patch)
2011-12-01 03:05 PST, alexander :surkov
mzehe: review+
Details | Diff | Splinter Review
patch: second round (2.30 KB, patch)
2012-03-20 19:03 PDT, alexander :surkov
tbsaunde+mozbugs: review+
bugzilla: approval‑mozilla‑aurora+
bugzilla: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Carsten Book [:Tomcat] 2011-12-01 02:54:26 PST
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
Comment 1 alexander :surkov 2011-12-01 03:05:58 PST
Created attachment 578219 [details] [diff] [review]
patch
Comment 2 Marco Zehe (:MarcoZ) on PTO until August 15 2011-12-01 03:11:40 PST
Comment on attachment 578219 [details] [diff] [review]
patch

r=me thanks for the quick fix!
Comment 3 Matt Brubeck (:mbrubeck) 2011-12-01 11:40:58 PST
https://hg.mozilla.org/mozilla-central/rev/768c2a7a320e
Comment 5 David Bolter [:davidb] 2011-12-16 06:29:11 PST
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)?
Comment 6 David Bolter [:davidb] 2011-12-16 07:18:19 PST
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.
Comment 7 David Bolter [:davidb] 2012-01-09 10:21:21 PST
Users are still getting crashes.
Comment 8 Scoobidiver (away) 2012-01-09 10:42:55 PST
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 Scoobidiver (away) 2012-01-13 10:25:21 PST
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
Comment 10 Marcia Knous [:marcia - use ni] 2012-02-07 14:10:36 PST
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 Scoobidiver (away) 2012-02-08 00:20:03 PST
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 Sheila Mooney 2012-02-13 09:22:28 PST
Marcia, did we try and reproduce this?
Comment 13 Scoobidiver (away) 2012-03-20 09:59:19 PDT
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
Comment 14 David Bolter [:davidb] 2012-03-20 12:43:53 PDT
Alexander, any ideas?
Comment 15 Trevor Saunders (:tbsaunde) 2012-03-20 13:56:50 PDT
(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
Comment 16 alexander :surkov 2012-03-20 16:17:03 PDT
a crash link pls?
Comment 17 David Bolter [:davidb] 2012-03-20 16:28:33 PDT
(In reply to alexander :surkov from comment #16)
> a crash link pls?

http://bit.ly/GCu1gq
Comment 18 alexander :surkov 2012-03-20 19:03:44 PDT
Created attachment 607832 [details] [diff] [review]
patch: second round
Comment 19 Trevor Saunders (:tbsaunde) 2012-03-22 20:33:10 PDT
Comment on attachment 607832 [details] [diff] [review]
patch: second round

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

false?
Comment 20 alexander :surkov 2012-03-22 20:40:30 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #19)

> >+  mIsOnclick = nsnull;
> 
> false?

sure, I noticed that after I filed the patch :)
Comment 21 alexander :surkov 2012-03-23 05:23:19 PDT
(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
Comment 22 Ed Morley [:emorley] 2012-03-24 14:12:42 PDT
https://hg.mozilla.org/mozilla-central/rev/21fabfdbeaab
Comment 23 Scoobidiver (away) 2012-03-24 23:41:03 PDT
It's #25 top browser crasher in 12.0b1 and 13.0a2.
That qualifies it for Aurora and Beta approvals.
Comment 24 alexander :surkov 2012-03-25 22:12:39 PDT
(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 Scoobidiver (away) 2012-03-25 23:48:17 PDT
(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
Comment 26 alexander :surkov 2012-03-29 06:40:03 PDT
it sounds like it improves things: two crashes for 25 and 28 March builds. Makes sense to back port this patch.
Comment 27 Marco Zehe (:MarcoZ) on PTO until August 15 2012-03-29 08:10:13 PDT
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
Comment 28 Johnathan Nightingale [:johnath] 2012-03-29 14:42:45 PDT
Comment on attachment 607832 [details] [diff] [review]
patch: second round

Discussed and approved in triage, land it soon please!
Comment 29 Marco Zehe (:MarcoZ) on PTO until August 15 2012-03-29 23:33:36 PDT
Landed on Alexander's behalf:
http://hg.mozilla.org/releases/mozilla-aurora/rev/fdce25e870da
http://hg.mozilla.org/releases/mozilla-beta/rev/833d79ffbf39
Comment 30 Michael Lefevre 2012-03-30 01:13:52 PDT
(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.
Comment 31 Marco Zehe (:MarcoZ) on PTO until August 15 2012-03-30 01:49:23 PDT
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.
Comment 32 alexander :surkov 2012-03-30 02:01:25 PDT
(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.
Comment 33 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-03 15:04:22 PDT
Verify fix by checking crashstats after the March 29th landing.
Comment 34 Virgil Dicu [:virgil] [QA] 2012-04-19 09:21:33 PDT
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 Robert Kaiser 2012-04-19 10:08:01 PDT
(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).
Comment 36 Virgil Dicu [:virgil] [QA] 2012-04-20 00:42:35 PDT
Thank you for the clarification, Robert. Marking this as verified on Firefox 12 based on crash-stats data.
Comment 37 Virgil Dicu [:virgil] [QA] 2012-05-09 08:01:51 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.