729861 - Crash in nsDocAccessible::GetAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Scoobidiver (away)]

It's #1 top crasher in 13.0a1 over the last day.
It first appeared in 13.0a1/20120222. The regression window is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=be559203ece8&tochange=373c710112e6
It's likely a regression from bug 726097.

Signature 	PL_DHashTableOperate | nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsAutoPtr<nsPreflightCache::CacheEntry> > >::GetEntry(nsACString_internal const&) More Reports Search
UUID	3ddb0c53-f67b-4950-af70-b28fa2120223
Date Processed	2012-02-23 08:24:25
Uptime	9
Last Crash	41 seconds before submission
Install Age	1.3 hours since version was first installed.
Install Time	2012-02-23 07:05:06
Product	Firefox
Version	13.0a1
Build ID	20120222031223
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 37 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0xc4
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0046, AdapterSubsysID: 1c221043, AdapterDriverVersion: 8.15.10.2202
D2D? D2D+
DWrite? DWrite+
D3D10 Layers? D3D10 Layers+
EMCheckCompatibility	True
Total Virtual Memory	4294836224
Available Virtual Memory	3882524672
System Memory Use Percentage	51
Available Page File	5306892288
Available Physical Memory	1959522304

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	PL_DHashTableOperate 	obj-firefox/xpcom/build/pldhash.cpp:615
1 	xul.dll 	nsTHashtable<nsBaseHashtableET<nsCStringHashKey,nsAutoPtr<nsPreflightCache::CacheEntry> > >::GetEntry 	obj-firefox/dist/include/nsTHashtable.h:170
2 	xul.dll 	nsBaseHashtable<nsUint32HashKey,mozilla::plugins::PluginIdentifierChildInt*,mozilla::plugins::PluginIdentifierChildInt*>::Get 	obj-firefox/dist/include/nsClassHashtable.h:140
3 	xul.dll 	nsDocAccessible::GetAccessible 	accessible/src/base/nsDocAccessible.cpp:593
4 	xul.dll 	nsDocAccessible::GetAccessibleOrContainer 	accessible/src/base/nsDocAccessible.cpp:1370
5 	xul.dll 	nsAccessible::ChildAtPoint 	accessible/src/base/nsAccessible.cpp:856
6 	ntdll.dll 	RtlFreeHeap 	
7 	xul.dll 	nsAccessibleWrap::accHitTest 	accessible/src/msaa/nsAccessibleWrap.cpp:890
8 	oleacc.dll 	AccWrap_Base::accHitTest 	
9 	oleacc.dll 	AccWrap_LocationEtcFix::accHitTest 	
10 	oleacc.dll 	AccessibleObjectFromPoint 	
11 	oleacc.dll 	EXTERNAL_AccessibleObjectFromPoint 	
12 	TextExtractorImpl32.dll 	TextExtractorImpl32.dll@0x6bcc

More reports at:
https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate%20|%20PL_DHashTableOperate%20|%20nsTHashtable%3CnsBaseHashtableET%3CnsStringHashKey%2C%20nsAutoPtr%3CnsTArray%3CnsAutoPtr%3CnsDocAccessible%3A%3AAttrRelProvider%3E%2C%20nsTArrayDefaultAllocator%3E%20%3E%20%3E%20%3E%3A%3AGetEntry%28nsAString_internal%20const%26%29
https://crash-stats.mozilla.com/report/list?signature=PL_DHashTableOperate%20|%20nsTHashtable%3CnsBaseHashtableET%3CnsCStringHashKey%2C%20nsAutoPtr%3CnsPreflightCache%3A%3ACacheEntry%3E%20%3E%20%3E%3A%3AGetEntry%28nsACString_internal%20const%26%29

Comment 1

[Marco Zehe (:MarcoZ)]

TextExtractorImpl32.dll is none of the known accessibility consumers. More details here: http://www.isthisfilesafe.com/sha1/45B5BDF282E3585F08B63BE368AE4068B3453559_details.aspx
Might make itharder to reproduce, unless Hub has an idea straight away from looking at the stack.

Comment 2

[Scoobidiver (away)]

TextExtractorImpl32.dll and TextExtractorImpl64.dll are parts of YouDao Dictionary (Chinese).
Kazahook.dll is part of Kazaguru (Japanese) (see http://www.staticflower.net/software/kazaguru.html).

Comment 3

[Hubert Figuiere [:hub]]

It might be more related to bug 672504

Like that I have no idea what it going on.

Comment 4

[Hubert Figuiere [:hub]]

I'm cooking up a patch, which I hope is the reason. Do we have an idea on how to reproduce?

Comment 5

[Hubert Figuiere [:hub]]

Attachment: Crash: make sure we don't get a nsnull doc accessible. r=

Comment 6

[Hubert Figuiere [:hub]]

Comment on attachment 600054 [details] [diff] [review]
Crash: make sure we don't get a nsnull doc accessible. r=

Can't really reproduce. But this could actually fix the crash. Question is why would content->OwnerDoc() be nsnull?

Comment 7

[Trevor Saunders (:tbsaunde)]

(In reply to Hub Figuiere [:hub] from comment #6)
> Comment on attachment 600054 [details] [diff] [review]
> Crash: make sure we don't get a nsnull doc accessible. r=
> 
> Can't really reproduce. But this could actually fix the crash. Question is
> why would content->OwnerDoc() be nsnull?

I believe the only way for that to be true is if we are in the middle of a cc, and I'm nto sure its possible for us to be running then...

another posibility would seem to be for us to not create an accessible document for the doc in question, but I'm not really clear on why this code was needed in the first place or why contentDoc isn't always the same as mDoc.

Comment 8

[Trevor Saunders (:tbsaunde)]

Comment on attachment 600054 [details] [diff] [review]
Crash: make sure we don't get a nsnull doc accessible. r=

Alex I think you'll have time to think deeply about this before me, so over to you.

Comment 9

[Hubert Figuiere [:hub]]

Try builds available at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-c069918dd57c/

If somebody who has experienced the crash can try this.

Comment 10

[alexander :surkov (:asurkov)]

Comment on attachment 600054 [details] [diff] [review]
Crash: make sure we don't get a nsnull doc accessible. r=

Review of attachment 600054 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/nsAccessible.cpp
@@ +853,5 @@
>    // the DOM parent chain.
>    nsDocAccessible* contentDocAcc = GetAccService()->
>      GetDocAccessible(content->OwnerDoc());
> +  nsAccessible* accessible = contentDocAcc ? 
> +    contentDocAcc->GetAccessibleOrContainer(content) : nsnull;

I'd add null check for contentDocAcc instead and return early

Comment 11

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #7)

> another posibility would seem to be for us to not create an accessible
> document for the doc in question,

that's right I think, sometimes we don't create an accessible for document when we should (I think this is the case)

> but I'm not really clear on why this code
> was needed in the first place or why contentDoc isn't always the same as
> mDoc.

I think at some point they changed logic at layout layer so they started to look deeply through subdocuments

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 600054 [details] [diff] [review]
Crash: make sure we don't get a nsnull doc accessible. r=

Review of attachment 600054 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/nsAccessible.cpp
@@ +853,5 @@
>    // the DOM parent chain.
>    nsDocAccessible* contentDocAcc = GetAccService()->
>      GetDocAccessible(content->OwnerDoc());
> +  nsAccessible* accessible = contentDocAcc ? 
> +    contentDocAcc->GetAccessibleOrContainer(content) : nsnull;

please assert when there's no document, maybe at some point somebody will be able to catch it

Comment 13

[alexander :surkov (:asurkov)]

(In reply to Scoobidiver from comment #2)
> TextExtractorImpl32.dll and TextExtractorImpl64.dll are parts of YouDao
> Dictionary (Chinese).
> Kazahook.dll is part of Kazaguru (Japanese) (see
> http://www.staticflower.net/software/kazaguru.html).

I filed bug 730198 to telemetry them. Btw, what is Kazaguru (I can't read Japanese)? google mentions it as MalWare.

Comment 14

[Hubert Figuiere [:hub]]

Adapted according to comment 12

landed on inbound 
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e07d54535af

Comment 15

[alexander :surkov (:asurkov)]

(In reply to Hub Figuiere [:hub] from comment #14)
> Adapted according to comment 12
> 
> landed on inbound 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6e07d54535af

it's not an assertion though, so if we get a null document then it won't be noticed likely.

Comment 16

[alexander :surkov (:asurkov)]

and usually we provide a bug number not the url

Comment 17

[Hubert Figuiere [:hub]]

I'll submit a new revision of the patch.

Comment 18

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/6e07d54535af

leaving open considered comment 17

Comment 19

[Hubert Figuiere [:hub]]

Attachment: Part 2: make the doc accessible check an assert. r=

Comment 20

[Hubert Figuiere [:hub]]

Part 2 inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/88ce402f3944

Comment 21

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/88ce402f3944