Closed Bug 649409 Opened 9 years ago Closed 9 years ago

Firefox 4.2a1pre Crash Report [@ nsLinkableAccessible::NativeState() ]

Categories

(Firefox :: Disability Access, defect)

x86
Windows Vista
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 653584

People

(Reporter: smooney, Assigned: tbsaunde)

References

Details

Attachments

(1 file)

New crash on the trunk appeared on April 11, 2011

Signature	nsLinkableAccessible::NativeState()
UUID	83f9ec81-5dec-42f1-803c-f8d392110412
Time 	2011-04-12 08:47:45.206690
Uptime	74
Last Crash	77 seconds before submission
Install Age	75585 seconds (21.0 hours) since version was first installed.
Product	Firefox
Version	4.2a1pre
Build ID	20110411030525
Branch	2.2
OS	Windows NT
OS Version	6.0.6002 Service Pack 2
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 13
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
User Comments	
App Notes 	AdapterVendorID: 10de, AdapterDeviceID: 0427, AdapterDriverVersion: 8.16.11.8766
D3D10 Layers? D3D10 Layers-
D3D9 Layers? D3D9 Layers-
Processor Notes 	
EMCheckCompatibility	False

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsLinkableAccessible::NativeState 	accessible/src/base/nsBaseWidgetAccessible.cpp:124
1 	xul.dll 	nsHTMLTextAccessible::NativeState 	accessible/src/html/nsHTMLTextAccessible.cpp:91
2 	xul.dll 	nsAccessible::State 	accessible/src/base/nsAccessible.cpp:1530
3 	xul.dll 	nsAccessibleWrap::get_accState 	accessible/src/msaa/nsAccessibleWrap.cpp:448

Crash Reports:
https://crash-stats.mozilla.com/report/list?product=Firefox&branch=2.2&platform=windows&build_id=20110411030525&query_search=signature&query_type=exact&query=&date=04%2F12%2F2011%2010%3A45%3A53&range_value=30&range_unit=days&hang_type=crash&process_type=browser&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsLinkableAccessible%3A%3ANativeState%28%29
OS: Mac OS X → Windows Vista
I suspect this is a dupe, or different incarnation, of bug 648988, which was fixed for the April 12 nightly builds. Watch out for it, but I believe this is fixed already.
maybe, but it can crash due to different reason: no action accessible (for generic text node accessible). Prior bug 634218 we had null check.

Trevor, please fix it?
Blocks: 634218
(In reply to comment #2)
> maybe, but it can crash due to different reason: no action accessible (for
> generic text node accessible). Prior bug 634218 we had null check.

This shouldn't happen because we are under mIsLink condition. So if that happens then something goes really wrong. I tend to agree with Marco comment #1.
Attachment #525609 - Flags: review?(mar.marcoz)
Attachment #525609 - Flags: review?(bolterbugz)
I'm not 100% sure GetActionAccessible can return NULL for a non defunct accessible but it seems pretty possible reading GetActionAccessible.
Assignee: nobody → trev.saunders
Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

r=me. I like the approach of returning early if we determine we don#t have anything else to do here.
Attachment #525609 - Flags: review?(mar.marcoz) → review+
Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

This seems ok to me, but I'm not sure if !mIsLink indicates the object simply hasn't been bound to its parent yet (in our tree) or if it indicates this is a linkable accessible that doesn't actually have a link in its ancestry chain. I'd like to understand the cause of this bug; let's get Alexander's thoughts as well.

>+    state |= actionAcc->State() & states::TRAVERSED;

This was a nice change.
Attachment #525609 - Flags: review?(bolterbugz) → review+
(In reply to comment #7)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> This seems ok to me, but I'm not sure if !mIsLink indicates the object simply
> hasn't been bound to its parent yet (in our tree) or if it indicates this is a
> linkable accessible that doesn't actually have a link in its ancestry chain.
> I'd like to understand the cause of this bug; let's get Alexander's thoughts as
> well.

sure,  note that  I didn't change what we did if mIsLink is false.  I'm not sure what it being false means, but I do know we crashed when it was true.  The crash has to do with actionAcc being null, which may be related to mIsLink though.
Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

Actually I'm not sure what state might hold at this point. Seems better to return 0 I think. Note intrinsic states don't map directly to our a11y states...

> PRUint64
> nsLinkableAccessible::NativeState()
> {

>+  PRUint64 state = nsAccessibleWrap::NativeState();
>+  if (!mIsLink)
>+    return state;
>+
(In reply to comment #9)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> Actually I'm not sure what state might hold at this point. Seems better to
> return 0 I think. Note intrinsic states don't map directly to our a11y
> states...

hmm, I'm not sure where your refering to now ;)
Actually I took a closer look at the base NativeState method and we don't just pass instrinsic states through of course, so don't worry about comment 9. Thanks.
(In reply to comment #8)
> (In reply to comment #7)
> > Comment on attachment 525609 [details] [diff] [review]
> > Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> > 
> > This seems ok to me, but I'm not sure if !mIsLink indicates the object simply
> > hasn't been bound to its parent yet (in our tree) or if it indicates this is a
> > linkable accessible that doesn't actually have a link in its ancestry chain.
> > I'd like to understand the cause of this bug; let's get Alexander's thoughts as
> > well.
> 
> sure,  note that  I didn't change what we did if mIsLink is false.  I'm not
> sure what it being false means, but I do know we crashed when it was true.  The
> crash has to do with actionAcc being null, which may be related to mIsLink
> though.

Weird. It is worth figuring out if you have time.
(In reply to comment #12)

> > sure,  note that  I didn't change what we did if mIsLink is false.  I'm not
> > sure what it being false means, but I do know we crashed when it was true.  The
> > crash has to do with actionAcc being null, which may be related to mIsLink
> > though.
> 
> Weird. It is worth figuring out if you have time.

Nothing weird here. If node is defunct then this scenario is possible. I don't see other scenarios. If you can point me it then it's great and I thumb up to take a patch.
Comment on attachment 525609 [details] [diff] [review]
Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]

r- to prevent the landing of this patch just in case. I don't see a scenario how it may be null. So if you are pretty sure we need to have null check here (in the case we've got completely broken tree) then please add an assertion since the !actionAcc condition is never true.
Attachment #525609 - Flags: review?(surkov.alexander) → review-
(In reply to comment #14)
> Comment on attachment 525609 [details] [diff] [review]
> Bug 649409 - crash in [@ nsLinkableAccessible::NativeState() ]
> 
> r- to prevent the landing of this patch just in case. I don't see a scenario
> how it may be null. So if you are pretty sure we need to have null check here
> (in the case we've got completely broken tree) then please add an assertion
> since the !actionAcc condition is never true.

Looking deeper I don't think it can be null if the object isn't defunct
Are we going to make any progress on this bug for Firefox 6?
(In reply to comment #16)
> Are we going to make any progress on this bug for Firefox 6?

I suspect this was infact a different form of bug  648988.  bug 653584 may be related, but I doubt it, I think that one is linux specific, but maybe not.  Anyway I don't think we've seen any crashes like this in a pretty good while so I think we can consider this  fixed  unless I've been missing some crashes :)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Trevor, if you mark the bug fixed then you should point what is a fix (patch or bug), otherwise worksforme.
(In reply to comment #18)
> Trevor, if you mark the bug fixed then you should point what is a fix (patch
> or bug), otherwise worksforme.

sure, sorry about that
Resolution: FIXED → WORKSFORME
Should this have been fixed in the 20110517 build? I saw it twice last night:
bp-7d2ee591-eacb-4d64-984f-006af2110518
bp-17852a35-244e-4185-87e8-661702110518
Unfortunately, I can't find the URL that caused the crash. Damn.
Reopening based on comment 20.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(In reply to comment #21)
> Reopening based on comment 20.

for me it sounds as a dupe of bug 653584
You need to log in before you can comment on or make changes to this bug.