If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Shutdown() of nsXULTooltipAccessible is not called

RESOLVED FIXED in mozilla1.9.1a1

Status

()

Core
Disability Access APIs
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Ginn Chen, Assigned: Evan Yan)

Tracking

({fixed1.9.0.2})

unspecified
mozilla1.9.1a1
x86
SunOS
fixed1.9.0.2
Points:
---
Bug Flags:
wanted1.9.1 +
blocking1.9.0.1 -
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [has-patch,has-review])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

10 years ago
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"

(Reporter)

Updated

10 years ago
Assignee: nobody → Evan.Yan
(Assignee)

Comment 2

10 years ago
Created attachment 321748 [details] [diff] [review]
don't create atkObject for accessible already shut down
Attachment #321748 - Flags: review?(ginn.chen)
(Reporter)

Comment 3

10 years ago
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-
(Assignee)

Comment 4

10 years ago
Created attachment 321752 [details] [diff] [review]
patch v2
Attachment #321748 - Attachment is obsolete: true
Attachment #321752 - Flags: review?(ginn.chen)
(Reporter)

Comment 5

10 years ago
Comment on attachment 321752 [details] [diff] [review]
patch v2

"nodes which have been" is better, I think.
Attachment #321752 - Flags: review?(ginn.chen) → review+
(Assignee)

Comment 6

10 years ago
Comment on attachment 321752 [details] [diff] [review]
patch v2

fix potential crash with low risk. only affect linux/unix.
Attachment #321752 - Flags: approval1.9?
(Reporter)

Updated

10 years ago
Whiteboard: [has-patch,has-review][RC2?]

Comment 7

10 years ago
Ginn, Evan, under what circumstances can Orca access these uncleaned ATK objects? Any specific steps to reproduce this one?
(Reporter)

Comment 8

10 years ago
Open several tabs, and close them one by one.

Comment 9

10 years ago
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?
(Assignee)

Comment 10

10 years ago
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

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

Comment 12

10 years ago
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-

Updated

10 years ago
Blocks: 391535
No longer blocks: 396346

Comment 14

10 years ago
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

10 years ago
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]
(Assignee)

Comment 17

9 years ago
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
Last Resolved: 9 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-
(Assignee)

Comment 21

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

Updated

9 years ago
Target Milestone: --- → mozilla1.9.1a1
(Assignee)

Updated

9 years ago
Keywords: fixed1.9.0.2
You need to log in before you can comment on or make changes to this bug.