Last Comment Bug 386914 - Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified event handler and observes
: Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified ev...
: crash, fixed1.8.0.15, testcase, verified1.8.1.8
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 Windows XP
-- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
: Neil Deakin
: 351347 (view as bug list)
Depends on:
Blocks: 351347
  Show dependency treegraph
Reported: 2007-07-04 17:30 PDT by Martijn Wargers [:mwargers]
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.1.x+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (423 bytes, application/vnd.mozilla.xul+xml)
2007-07-04 17:30 PDT, Martijn Wargers [:mwargers]
no flags Details
proposed patch (3.32 KB, patch)
2007-07-05 03:28 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
jst: review+
jst: superreview+
Details | Diff | Splinter Review
for 1.8 (3.00 KB, patch)
2007-07-16 02:16 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
dveditz: approval1.8.1.8+
Details | Diff | Splinter Review

Description User image Martijn Wargers [:mwargers] 2007-07-04 17:30:15 PDT
Created attachment 270976 [details]

See testcase, which crashes Mozilla within 100ms.
It also crashes branch, so marking security sensitive for now.
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.
Comment 2 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-05 00:55:01 PDT
Though, in this case the crash is not because of that, but because 
BroadcastListener gets deleted, I think.
Patch coming.
Comment 3 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-05 01:03:13 PDT
Patch is not coming yet. I need to think broadcaster a bit more.
There are other possible crashers there, I think.
Comment 4 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-05 03:28:17 PDT
Created attachment 271019 [details] [diff] [review]
proposed patch

For this type of crash keeping broadcast listeners alive is needed.
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2007-07-05 10:25:28 PDT
This is basically the same as bug 351347, right?  Down to the stack, and the analysis, at least... ;)
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2007-07-05 10:26:28 PDT
Note, in particular, the comments in that bug explaining why calling SetAttr here is bad to start with.
Comment 7 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-05 11:22:35 PDT
Yes, looks like the same bug. The suggestion in comment 6 in that bug
sounds pretty similar to what I did.
Comment 8 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-07-16 02:16:00 PDT
Created attachment 272465 [details] [diff] [review]
for 1.8
Comment 9 User image Martijn Wargers [:mwargers] 2007-07-19 14:21:01 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
Comment 10 User image Daniel Veditz [:dveditz] 2007-08-01 19:53:24 PDT
deleted aListener passed to nsXULDocument::ExecuteOnBroadcastHandlerFor(), crashes and
Comment 11 User image Adam Guthrie 2007-08-02 10:51:25 PDT
*** Bug 351347 has been marked as a duplicate of this bug. ***
Comment 12 User image Daniel Veditz [:dveditz] 2007-08-29 15:27:47 PDT
Comment on attachment 272465 [details] [diff] [review]
for 1.8

approved for, a=dveditz for release-drivers
Comment 13 User image Carsten Book [:Tomcat] 2007-08-30 09:33:01 PDT
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2007083003 BonEcho/ and the testcase in this bug.
No crash on testcase - adding verified keyword.
Comment 14 User image Alexander Sack 2008-02-28 06:56:24 PST
Comment on attachment 272465 [details] [diff] [review]
for 1.8

a=asac for

(same patch shipped by distros for some time)
Comment 15 User image Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 09:32:50 PDT
fix committed to 1.8.0 branch
Comment 16 User image Bob Clary [:bc:] 2009-04-24 10:41:04 PDT
crash test landed

Note You need to log in before you can comment on or make changes to this bug.