Last Comment Bug 386914 - Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified event handler and observes
: Crash [@ nsXULDocument::ExecuteOnBroadcastHandlerFor] with DOMAttrModified ev...
Status: VERIFIED FIXED
[sg:critical?]
: 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]
:
Mentors:
: 351347 (view as bug list)
Depends on:
Blocks: 351347
  Show dependency treegraph
 
Reported: 2007-07-04 17:30 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
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: ---


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

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2007-07-04 17:30:15 PDT
Created attachment 270976 [details]
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.
Comment 2 Olli Pettay [:smaug] 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 Olli Pettay [:smaug] 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 Olli Pettay [:smaug] 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 Boris Zbarsky [:bz] 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 Boris Zbarsky [:bz] 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 Olli Pettay [:smaug] 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 Olli Pettay [:smaug] 2007-07-16 02:16:00 PDT
Created attachment 272465 [details] [diff] [review]
for 1.8
Comment 9 Martijn Wargers [:mwargers] (not working for Mozilla) 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 Daniel Veditz [:dveditz] 2007-08-01 19:53:24 PDT
deleted aListener passed to nsXULDocument::ExecuteOnBroadcastHandlerFor(), crashes 1.8.0.12 and 1.8.1.6
Comment 11 Adam Guthrie 2007-08-02 10:51:25 PDT
*** Bug 351347 has been marked as a duplicate of this bug. ***
Comment 12 Daniel Veditz [:dveditz] 2007-08-29 15:27:47 PDT
Comment on attachment 272465 [details] [diff] [review]
for 1.8

approved for 1.8.1.7, a=dveditz for release-drivers
Comment 13 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:1.8.1.7pre) Gecko/2007083003 BonEcho/2.0.0.7pre and the testcase in this bug.
No crash on testcase - adding verified keyword.
Comment 14 Alexander Sack 2008-02-28 06:56:24 PST
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)
Comment 15 Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 09:32:50 PDT
fix committed to 1.8.0 branch
Comment 16 Bob Clary [:bc:] 2009-04-24 10:41:04 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/9a7346ed943a

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