Closed Bug 432970 Opened 14 years ago Closed 14 years ago

Shutdown() of nsXULTooltipAccessible is not called

Categories

(Core :: Disability Access APIs, defect)

x86
SunOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.1a1

People

(Reporter: ginnchen+exoracle, Assigned: evan.yan)

References

Details

(Keywords: fixed1.9.0.2, Whiteboard: [has-patch,has-review])

Attachments

(1 file, 1 obsolete file)

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.
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"

Assignee: nobody → Evan.Yan
Attachment #321748 - Flags: review?(ginn.chen)
Comment on attachment 321748 [details] [diff] [review]
don't create atkObject for accessible already shut down

Move into "if (!mAtkObject) {}" please.
Attachment #321748 - Flags: review?(ginn.chen) → review-
Attached patch patch v2Splinter Review
Attachment #321748 - Attachment is obsolete: true
Attachment #321752 - Flags: review?(ginn.chen)
Comment on attachment 321752 [details] [diff] [review]
patch v2

"nodes which have been" is better, I think.
Attachment #321752 - Flags: review?(ginn.chen) → review+
Comment on attachment 321752 [details] [diff] [review]
patch v2

fix potential crash with low risk. only affect linux/unix.
Attachment #321752 - Flags: approval1.9?
Whiteboard: [has-patch,has-review][RC2?]
Ginn, Evan, under what circumstances can Orca access these uncleaned ATK objects? Any specific steps to reproduce this one?
Open several tabs, and close them one by one.
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?
Flags: blocking1.9.0.1?
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.
I guess so.

Thank you very much, Marco.
Whiteboard: [has-patch,has-review][RC2?] → [has-patch,has-review][RC2-]
Flags: blocking1.9.0.1? → blocking1.9.0.1+
Comment on attachment 321752 [details] [diff] [review]
patch v2

- for FF3, we'll take this in 3.0.1
Attachment #321752 - Flags: approval1.9? → approval1.9-
Blocks: 191a11y
No longer blocks: fox3access
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 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.
Attachment #321752 - Flags: approval1.9.0.1?
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?
Flags: wanted1.9.1+
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1+
Whiteboard: [has-patch,has-review][RC2-] → [has-patch,has-review]
I didn't see any regression. Marco, have you ever seen one?
I also am not seeing any regressions when running with this patch.
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.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
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.
Attachment #321752 - Flags: approval1.9.0.2+
Attachment #321752 - Flags: approval1.9.0.1?
Attachment #321752 - Flags: approval1.9.0.1-
Checking in nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/atk/nsAccessibleWrap.cpp,v  <--  nsAccessibleWrap.cpp
new revision: 1.111; previous revision: 1.110
done
Target Milestone: --- → mozilla1.9.1a1
Keywords: fixed1.9.0.2
You need to log in before you can comment on or make changes to this bug.