Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified event handler and observes

VERIFIED FIXED

Status

()

defect
--
critical
VERIFIED FIXED
12 years ago
8 years ago

People

(Reporter: martijn.martijn, Assigned: smaug)

Tracking

(4 keywords)

Trunk
x86
Windows XP
Points:
---
Bug Flags:
blocking1.8.1.8 +
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Posted file testcase
See testcase, which crashes Mozilla within 100ms.
It also crashes branch, so marking security sensitive for now.

http://crash-stats.mozilla.com/report/index/e7598a70-2a8c-11dc-952e-001a4bd43e5c
0  	@0x2436041
1 	nsCOMPtr<nsIContent>::nsCOMPtr<nsIContent>(nsQueryInterface)
2 	nsXULDocument::ExecuteOnBroadcastHandlerFor(nsIContent *,nsIDOMElement *,nsIAtom *)
3 	nsXULDocument::AttributeChanged(nsIDocument *,nsIContent *,int,nsIAtom *,int)
4 	nsNodeUtils::AttributeChanged(nsIContent *,int,nsIAtom *,int)
5 	nsGenericElement::SetAttrAndNotify(int,nsIAtom *,nsIAtom *,nsAString_internal const &,nsAttrValue &,int,int,int)
6 	nsGenericElement::SetAttr(int,nsIAtom *,nsIAtom *,nsAString_internal const &,int)
7 	nsGenericElement::SetAttribute(nsAString_internal const &,nsAString_internal const &)
8 	NS_InvokeByIndex_P
9 	AutoJSSuspendRequest::SuspendRequest()

Branch talkback ID: TB33739202G

I think this might be related to bug 351347.
(Assignee)

Comment 2

12 years ago
Though, in this case the crash is not because of that, but because 
BroadcastListener gets deleted, I think.
Patch coming.
Assignee: jag → Olli.Pettay
(Assignee)

Comment 3

12 years ago
Patch is not coming yet. I need to think broadcaster a bit more.
There are other possible crashers there, I think.
(Assignee)

Comment 4

12 years ago
For this type of crash keeping broadcast listeners alive is needed.
Attachment #271019 - Flags: superreview?(jst)
Attachment #271019 - Flags: review?(jst)
This is basically the same as bug 351347, right?  Down to the stack, and the analysis, at least... ;)
Note, in particular, the comments in that bug explaining why calling SetAttr here is bad to start with.
(Assignee)

Comment 7

12 years ago
Yes, looks like the same bug. The suggestion in comment 6 in that bug
sounds pretty similar to what I did.
Attachment #271019 - Flags: superreview?(jst)
Attachment #271019 - Flags: superreview+
Attachment #271019 - Flags: review?(jst)
Attachment #271019 - Flags: review+
(Assignee)

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: in-testsuite?
(Assignee)

Comment 8

12 years ago
Posted patch for 1.8Splinter Review
Attachment #272465 - Flags: approval1.8.1.6?
(Reporter)

Comment 9

12 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
Status: RESOLVED → VERIFIED
deleted aListener passed to nsXULDocument::ExecuteOnBroadcastHandlerFor(), crashes 1.8.0.12 and 1.8.1.6
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?]

Updated

12 years ago
Duplicate of this bug: 351347
Flags: blocking1.8.1.7+
Comment on attachment 272465 [details] [diff] [review]
for 1.8

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #272465 - Flags: approval1.8.1.7? → approval1.8.1.7+
(Assignee)

Updated

12 years ago
Keywords: fixed1.8.1.7
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7pre) Gecko/2007083003 BonEcho/2.0.0.7pre and the testcase in this bug.
No crash on testcase - adding verified keyword.
Group: security

Comment 14

11 years ago
Comment on attachment 272465 [details] [diff] [review]
for 1.8

a=asac for 1.8.0.15

(same patch shipped by distros for some time)
Attachment #272465 - Flags: approval1.8.0.15+
fix committed to 1.8.0 branch
Keywords: fixed1.8.0.15

Comment 16

10 years ago
crash test landed
http://hg.mozilla.org/mozilla-central/rev/9a7346ed943a
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsXULDocument::ExecuteOnBroadcastHandlerFor]
You need to log in before you can comment on or make changes to this bug.