A11y test suite crash [@ nsTArray_base::Length]

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: davidb, Assigned: surkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Had a crash during our a11y test suite run on:
WINNT 5.2 mozilla-central debug test mochitest-other Started 21:38, finished 22:03
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268015881.1268017337.9647.gz&fulltext=1#err0

 0  xul.dll!nsTArray_base::Length() [nsTArray.h:ee9dd725d8ea : 66 + 0x5]
    eip = 0x1000387c   esp = 0x0012c82c   ebp = 0x0012c830   ebx = 0x00000001
    esi = 0x00000c00   edi = 0x00000000   eax = 0x0775144c   ecx = 0xdddddddd
    edx = 0x019a3e8c   efl = 0x00210202
    Found by: given as instruction pointer in context
 1  xul.dll!nsTArray<nsRefPtr<nsAccessible> >::Clear() [nsTArray.h:ee9dd725d8ea : 707 + 0x7]
    eip = 0x10eed72f   esp = 0x0012c838   ebp = 0x0012c83c
    Found by: call frame info
 2  xul.dll!nsAccessible::InvalidateChildren() [nsAccessible.cpp:ee9dd725d8ea : 2891 + 0xa]
    eip = 0x10eec2c4   esp = 0x0012c844   ebp = 0x0012c854
    Found by: call frame info
 3  xul.dll!nsAccessible::Shutdown() [nsAccessible.cpp:ee9dd725d8ea : 492 + 0x1d]
    eip = 0x10ee4346   esp = 0x0012c85c   ebp = 0x0012c864
    Found by: call frame info
 4  xul.dll!ClearCacheEntry [nsAccessNode.cpp:ee9dd725d8ea : 718 + 0x17]
    eip = 0x10ec9dc6   esp = 0x0012c86c   ebp = 0x0012c870
    Found by: call frame info
 5  xul.dll!nsBaseHashtable<nsPtrHashKey<void const >,nsCOMPtr<nsAccessNode>,nsAccessNode *>::s_EnumStub(PLDHashTable *,PLDHashEntryHdr *,unsigned int,void *) [nsBaseHashtable.h:ee9dd725d8ea : 346 + 0x1d]
    eip = 0x10eca5d0   esp = 0x0012c878   ebp = 0x0012c88c
    Found by: call frame info
 6  xul.dll!PL_DHashTableEnumerate [pldhash.c:ee9dd725d8ea : 754 + 0x18]
    eip = 0x11005468   esp = 0x0012c894   ebp = 0x0012c8d4
    Found by: call frame info
 7  xul.dll!nsBaseHashtable<nsPtrHashKey<void const >,nsCOMPtr<nsAccessNode>,nsAccessNode *>::Enumerate(PLDHashOperator (*)(void const *,nsCOMPtr<nsAccessNode> &,void *),void *) [nsBaseHashtable.h:ee9dd725d8ea : 221 + 0x11]
    eip = 0x10eca2d2   esp = 0x0012c8dc   ebp = 0x0012c8f4
    Found by: call frame info
 8  xul.dll!nsAccessNode::ClearCache(nsInterfaceHashtable<nsPtrHashKey<void const >,nsAccessNode> &) [nsAccessNode.cpp:ee9dd725d8ea : 726 + 0xe]
    eip = 0x10ec9d62   esp = 0x0012c8fc   ebp = 0x0012c904
    Found by: call frame info
 9  xul.dll!nsDocAccessible::Shutdown() [nsDocAccessible.cpp:ee9dd725d8ea : 650 + 0xe]
    eip = 0x10f0c174   esp = 0x0012c90c   ebp = 0x0012c924
    Found by: call frame info
10  xul.dll!nsRootAccessible::HandleEventWithTarget(nsIDOMEvent *,nsIDOMNode *) [nsRootAccessible.cpp:ee9dd725d8ea : 628 + 0x23]
    eip = 0x10f3cd27   esp = 0x0012c92c   ebp = 0x0012cc9c
    Found by: call frame info
11  xul.dll!nsRootAccessible::HandleEvent(nsIDOMEvent *) [nsRootAccessible.cpp:ee9dd725d8ea : 589 + 0x1a]
    eip = 0x10f3cb85   esp = 0x0012cca4   ebp = 0x0012ccbc
    Found by: call frame info
12  xul.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct *,nsIDOMEventListener *,nsIDOMEvent *,nsPIDOMEventTarget *,unsigned int,nsCxPusher *) [nsEventListenerManager.cpp:ee9dd725d8ea : 1082 + 0x11]
    eip = 0x103b02cf   esp = 0x0012ccc4   ebp = 0x0012ce70
    Found by: call frame info
13  xul.dll!nsEventListenerManager::HandleEvent(nsPresContext *,nsEvent *,nsIDOMEvent * *,nsPIDOMEventTarget *,unsigned int,nsEventStatus *,nsCxPusher *) [nsEventListenerManager.cpp:ee9dd725d8ea : 1198 + 0x26]
    eip = 0x103b0739   esp = 0x0012ce78   ebp = 0x0012cee0
    Found by: call frame info
14  xul.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor &,unsigned int,int,nsCxPusher *) [nsEventDispatcher.cpp:ee9dd725d8ea : 201 + 0x47]
    eip = 0x1050dbc9   esp = 0x0012cee8   ebp = 0x0012cf18
    Found by: call frame info
15  xul.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor &,unsigned int,nsDispatchingCallback *,int,nsCxPusher *) [nsEventDispatcher.cpp:ee9dd725d8ea : 296 + 0x3d]
    eip = 0x1050d7d9   esp = 0x0012cf20   ebp = 0x0012cf58
    Found by: call frame info
16  xul.dll!nsEventDispatcher::Dispatch(nsISupports *,nsPresContext *,nsEvent *,nsIDOMEvent *,nsEventStatus *,nsDispatchingCallback *,nsCOMArray<nsPIDOMEventTarget> *) [nsEventDispatcher.cpp:ee9dd725d8ea : 601 + 0x1d]
    eip = 0x1050e56e   esp = 0x0012cf60   ebp = 0x0012d084
    Found by: call frame info
17  xul.dll!nsEventDispatcher::DispatchDOMEvent(nsISupports *,nsEvent *,nsIDOMEvent *,nsPresContext *,nsEventStatus *) [nsEventDispatcher.cpp:ee9dd725d8ea : 664 + 0x1c]
    eip = 0x1050ecea   esp = 0x0012d08c   ebp = 0x0012d0d4
    Found by: call frame info
18  xul.dll!nsDocument::DispatchPageTransition(nsPIDOMEventTarget *,nsAString_internal const &,int) [nsDocument.cpp:ee9dd725d8ea : 7154 + 0x17]
    eip = 0x103ea792   esp = 0x0012d0dc   ebp = 0x0012d128
    Found by: call frame info
19  xul.dll!nsDocument::OnPageHide(int,nsIDOMEventTarget *) [nsDocument.cpp:ee9dd725d8ea : 7260 + 0x24]
    eip = 0x103eab49   esp = 0x0012d130   ebp = 0x0012d17c
    Found by: call frame info
20  xul.dll!DocumentViewerImpl::PageHide(int) [nsDocumentViewer.cpp:ee9dd725d8ea : 1263 + 0x29]
    eip = 0x103b62f3   esp = 0x0012d184   ebp = 0x0012d1d0
    Found by: call frame info
21  xul.dll!nsDocShell::FirePageHideNotification(int) [nsDocShell.cpp:ee9dd725d8ea : 1461 + 0x22]
    eip = 0x10b96437   esp = 0x0012d1d8   ebp = 0x0012d228
    Found by: call frame info
This happened on the next run as well:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268035779.1268037468.17464.gz
WINNT 5.2 mozilla-central debug test mochitest-other on 2010/03/07 18:38:01
s: win32-slave16

Is it possible it could be connected to the checkin of bug 548291?
Oops, sorry, copied from wrong log. Comment #1 was actually about:

WINNT 5.2 mozilla-central debug test mochitest-other on 2010/03/08 00:09:39
s: mw32-ix-slave04

(The link is correct.)
(In reply to comment #1)
> Is it possible it could be connected to the checkin of bug 548291?

Seems likely, let's wait for one more run . Note I haven't seen this fail on the Windows opt test machine (nor on try).
This is happening consistently on windows debug. I'll probably back out. Waiting for my local build to finish to see if I can recreate on my machine.
Note there is similarity with stack on bug 549715.
Could be mChildren has been already cycle collected when you try to clear it on ::Shutdown()? it is part of the collection.
Blocks: 548291

Comment 7

8 years ago
http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox&errorparser=unittest&logfile=1268068419.1268071104.21332.gz&buildtime=1268068419&buildname=WINNT%205.2%20mozilla-central%20debug%20test%20mochitest-other&fulltext=1
WINNT 5.2 mozilla-central debug test mochitest-other on 2010/03/08 09:13:39
(In reply to comment #6)
> Could be mChildren has been already cycle collected when you try to clear it on
> ::Shutdown()? it is part of the collection.

Seems possible yes.
Ah, I think I found the problem. I'll post a new patch when I have a bit of time, over on bug 548291.
(In reply to comment #9)
> Ah, I think I found the problem. I'll post a new patch when I have a bit of
> time, over on bug 548291.

I retract.
Note this crash exists even without the source code changes from my original push (on bug 548291). That is: the tests will crash trunk on windows.
(Assignee)

Comment 12

8 years ago
Created attachment 437519 [details] [diff] [review]
patch

1) more area and image map cleanup
2) do not create temporary imagemap accessible when area accessible is created so that area accessibles have correct parent what fix the crash in the end
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #437519 - Flags: review?(marco.zehe)
Attachment #437519 - Flags: review?(bolterbugz)
(Assignee)

Comment 13

8 years ago
try-server build will be available here - http://build.mozilla.org/tryserver-builds/surkov.alexander@gmail.com-try-a5ed21ed1d69
Comment on attachment 437519 [details] [diff] [review]
patch

Seeing no difference between regular nightly and try-server in rendering image maps, also no crashes. r=me for the manual testing aspect. I also browsed the patch and didn't see anything that looked out of place.
Attachment #437519 - Flags: review?(marco.zehe) → review+
Comment on attachment 437519 [details] [diff] [review]
patch

r=me, thanks! You have put my thoughts into code!

>+    // Not the main content for this frame. This happens because <area>
>+    // elements return the image frame as their primary frame. The main content
>+    // for the image frame is the image content.
>+    return GetAreaAccessible(weakFrame.GetFrame(), aNode, aWeakShell);

Nit: Maybe comment that this returns null if not an nsIImageFrame.
Attachment #437519 - Flags: review?(bolterbugz) → review+
(Assignee)

Comment 16

8 years ago
(In reply to comment #15)
> (From update of attachment 437519 [details] [diff] [review])
> r=me, thanks! You have put my thoughts into code!

yw! if I could read your thoughts I would find a problem much quicker :)

> >+    // Not the main content for this frame. This happens because <area>
> >+    // elements return the image frame as their primary frame. The main content
> >+    // for the image frame is the image content.
> >+    return GetAreaAccessible(weakFrame.GetFrame(), aNode, aWeakShell);
> 
> Nit: Maybe comment that this returns null if not an nsIImageFrame.

sure
(In reply to comment #16)
> (In reply to comment #15)
> > (From update of attachment 437519 [details] [diff] [review] [details])
> > r=me, thanks! You have put my thoughts into code!
> 
> yw! if I could read your thoughts I would find a problem much quicker :)

You trust my thoughts more than I do :)
(In reply to comment #14)
> (From update of attachment 437519 [details] [diff] [review])
> Seeing no difference between regular nightly and try-server in rendering image
> maps, also no crashes. r=me for the manual testing aspect. I also browsed the
> patch and didn't see anything that looked out of place.

Oh hmmm so we haven't confirmed this fixes the crash?
(Assignee)

Comment 19

8 years ago
(In reply to comment #18)

> Oh hmmm so we haven't confirmed this fixes the crash?

I do :) It doesn't crash for me any more. It makes sense if you'll try as well.
Ha! I mid-air collided.

Was: I wanted to confirm this fix tonight but my windows vm is too painful (need more RAM). I can certainly check on Friday (I'm away tomorrow). Either way I'm happy with the patch.

Now: Good enough! Thanks!
(Assignee)

Comment 21

8 years ago
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/d294c76984c6
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
For the record: I never experienced any crash with image maps, the first I heard of this crash was when David wanted to land bug 548291. I could only confirm that the a11y worked as before.
(Assignee)

Comment 23

8 years ago
I was able to reproduce the crash with David's mochitests only from bug bug 548291. I think it might be a crash that's hard to get in real life.
BTW: https://bugzilla.mozilla.org/show_bug.cgi?id=551155#c5 (but Julian says it is harmless?)

Comment 25

8 years ago
we believe so ;-)
You need to log in before you can comment on or make changes to this bug.