432970 - Shutdown() of nsXULTooltipAccessible is not called

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Ginn Chen]

With debug build on UNIX, open several tabs, and close them.

I got these assertions.
###!!! ASSERTION: ShutdownAtkObject() is not called: '!mAtkObject', file nsAccessibleWrap.cpp, line 307

This may cause unexpected behavior including crash, if AT accesses the uncleaned atk object.

Comment 1

[Ginn Chen]

call stack:

=>[1] nsAccessibleWrap::~nsAccessibleWrap(this = 0xf3dcc420), line 306 in "nsAccessibleWrap.cpp"
  [2] nsLeafAccessible::~nsLeafAccessible(0xf3dcc420, 0x0), at 0xf942b7dc 
  [3] nsXULTooltipAccessible::~nsXULTooltipAccessible(0xf3dcc420, 0x0), at 0xf942edbc 
  [4] __SLIP.DELETER__B(0xf3dcc420, 0x1), at 0xf942c9fc 
  [5] nsAccessNode::LastRelease(this = 0xf3dcc420), line 146 in "nsAccessNode.cpp"
  [6] nsAccessNode::Release(this = 0xf3dcc420), line 120 in "nsAccessNode.cpp"
  [7] nsAccessible::Release(this = 0xf3dcc420), line 147 in "nsAccessible.cpp"
  [8] nsLeafAccessible::Release(this = 0xf3dcc420), line 66 in "nsBaseWidgetAccessible.cpp"
  [9] nsCOMPtr_base::~nsCOMPtr_base(this = 0xf018c208), line 81 in "nsCOMPtr.cpp"
  [10] nsCOMPtr<nsIAccessible>::~nsCOMPtr(0xf018c208, 0x0), at 0xf93096ec 
  [11] nsAccEvent::~nsAccEvent(this = 0xf018c1f0), line 94 in "nsAccessibleEventData.h"
  [12] __SLIP.DELETER__A(0xf018c1f0, 0x1), at 0xf930976c 
  [13] nsAccEvent::Release(this = 0xf018c1f0), line 60 in "nsAccessibleEventData.cpp"
  [14] ReleaseObjects(aElement = 0xf018c1f0, _ARG2 = (nil)), line 151 in "nsCOMArray.cpp"
  [15] nsVoidArray::EnumerateForwards(this = 0xf426f800, aFunc = 0xfead09e0 = &`libxpcom_core.so`nsCOMArray.cpp`ReleaseObjects(void*,void*), aData = (nil)), line 678 in "nsVoidArray.cpp"
  [16] nsCOMArray_base::Clear(this = 0xf426f800), line 158 in "nsCOMArray.cpp"
  [17] nsCOMArray<nsIAccessibleEvent>::Clear(this = 0xf426f800), line 217 in "nsCOMArray.h"
  [18] nsDocAccessible::FlushPendingEvents(this = 0xf426f760), line 1663 in "nsDocAccessible.cpp"
  [19] nsDocAccessible::FlushEventsCallback(aTimer = 0xf40aca40, aClosure = 0xf426f7b4), line 1680 in "nsDocAccessible.cpp"

Comment 2

[Evan Yan]

Attachment: don't create atkObject for accessible already shut down

Comment 3

[Ginn Chen]

Comment on attachment 321748 [details] [diff] [review]
don't create atkObject for accessible already shut down

Move into "if (!mAtkObject) {}" please.

Comment 4

[Evan Yan]

Attachment: patch v2

Comment 5

[Ginn Chen]

Comment on attachment 321752 [details] [diff] [review]
patch v2

"nodes which have been" is better, I think.

Comment 6

[Evan Yan]

Comment on attachment 321752 [details] [diff] [review]
patch v2

fix potential crash with low risk. only affect linux/unix.

Comment 7

[Marco Zehe (:MarcoZ)]

Ginn, Evan, under what circumstances can Orca access these uncleaned ATK objects? Any specific steps to reproduce this one?

Comment 8

[Ginn Chen]

Open several tabs, and close them one by one.

Comment 9

[Marco Zehe (:MarcoZ)]

Ginn, can you give me an estimate on what the risk is that this causes serious accessible tree corruption? Or is it more like amemory leak type of thing?

Comment 10

[Evan Yan]

It's not only memory leak. As described in comment #0
"
This may cause unexpected behavior including crash, if AT accesses the uncleaned atk object.
"

The patch make us not create atk object when mWeakShell == null, which I can only see when the nod has been shut down. So I think the risk is rather low.

Comment 11

[Marco Zehe (:MarcoZ)]

Are these two crash reports related?
bp-1232ae8a-2713-11dd-9485-001321b13766
and
bp-ee5c5aa8-268c-11dd-9758-001a4bd46e84

Comment 12

[Ginn Chen]

I guess so.

Thank you very much, Marco.

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 321752 [details] [diff] [review]
patch v2

- for FF3, we'll take this in 3.0.1

Comment 14

[Marco Zehe (:MarcoZ)]

Landed on mozilla-central in changeset:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/ff946562c355

Keeping bug open for landing on 1.9.0.1.

Comment 15

[Marco Zehe (:MarcoZ)]

Comment on attachment 321752 [details] [diff] [review]
patch v2

Requesting approval for 1.9.0.1 based on blocking + status, and on the fact that this can happen simply by opening and closing a tab in the Linux version of Firefox.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Not blocking 1.9.0.1, but will look at the patch approval shortly. I take it that we've seen no regressions on mozilla-central since it landed back on the 10th?

Comment 17

[Evan Yan]

I didn't see any regression. Marco, have you ever seen one?

Comment 18

[Marco Zehe (:MarcoZ)]

I also am not seeing any regressions when running with this patch.

Comment 19

[Marco Zehe (:MarcoZ)]

Setting to fixed in accordance with rules laid out by Sam in mozilla.dev.planning. Fix landed on mozilla-central on June 10th, see comment #14.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 321752 [details] [diff] [review]
patch v2

This missed 1.9.0.1. Let's get this in when the tree opens to checkins for 1.9.0.2.

Comment 21

[Evan Yan]

Checking in nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/atk/nsAccessibleWrap.cpp,v  <--  nsAccessibleWrap.cpp
new revision: 1.111; previous revision: 1.110
done