Last Comment Bug 375399 - Crash [@ nsElementSH::PostCreate] when removing window when accessing xul:tree in xul:tabs onselect in svg:foreignObject
: Crash [@ nsElementSH::PostCreate] when removing window when accessing xul:tre...
Status: VERIFIED FIXED
[sg:critical?] using deleted memory?
: crash, testcase, verified1.8.0.13, verified1.8.1.5
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.9alpha5
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 344486
  Show dependency treegraph
 
Reported: 2007-03-26 06:35 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2011-06-13 10:01 PDT (History)
13 users (show)
dveditz: blocking1.8.1.5+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.13+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
iframe needed for testcase (513 bytes, application/xhtml+xml)
2007-03-26 06:35 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
testcase (300 bytes, text/html)
2007-03-26 06:37 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Branch log from 2.0.0.4 RC1 (25.20 KB, text/plain)
2007-05-03 17:43 PDT, Samuel Sidler (old account; do not CC)
no flags Details
Mac Trunk log from 2007-04-28 (26.21 KB, text/plain)
2007-05-03 17:45 PDT, Samuel Sidler (old account; do not CC)
no flags Details
Mac Branch Debug log from 1.8 branch (~2007050317) (30.62 KB, text/plain)
2007-05-03 18:42 PDT, Samuel Sidler (old account; do not CC)
no flags Details
Does this fix the crash? (2.32 KB, patch)
2007-05-04 01:26 PDT, Boris Zbarsky [:bz]
bugs: review+
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13+
Details | Diff | Splinter Review
Same as diff -w for ease of review (1.61 KB, patch)
2007-05-04 01:39 PDT, Boris Zbarsky [:bz]
jst: superreview+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2007-03-26 06:35:31 PDT
Created attachment 259678 [details]
iframe needed for testcase

See upcoming testcase, which crashes current trunk and branch builds.

Talkback ID: TB30587921Q
0x02a8e765
nsElementSH::PostCreate  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 6836]
XPCWrappedNative::GetNewOrUsed  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 541]
XPCConvert::NativeInterface2JSObject  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 1104]
nsXPConnect::WrapNative  [mozilla/js/src/xpconnect/src/nsxpconnect.cpp, line 996]
nsDOMClassInfo::WrapNative  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 1484]
nsNodeSH::PreCreate  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 6388]
XPCWrappedNative::GetNewOrUsed  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 398]
XPCConvert::NativeInterface2JSObject  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 1104]
XPCConvert::NativeData2JS  [mozilla/js/src/xpconnect/src/xpcconvert.cpp, line 480]
XPCWrappedNative::CallMethod  [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2337]

Talkback ID for branch: TB30587979K
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-03-26 06:37:14 PDT
Created attachment 259679 [details]
testcase
Comment 2 Olli Pettay [:smaug] (vacation Aug 25-28) 2007-03-26 07:19:59 PDT
Note, only non-debug-builds crash.
Comment 3 Olli Pettay [:smaug] (vacation Aug 25-28) 2007-03-26 08:14:05 PDT
hmm, this wasn't the kind of crash I thought.
I'll look at this anyway, but if someone notices immediately what is
going wrong, feel free to take this.
Comment 4 Olli Pettay [:smaug] (vacation Aug 25-28) 2007-03-26 12:29:10 PDT
So mRuleNode is deleted before nsStyleContext?
Comment 5 Samuel Sidler (old account; do not CC) 2007-05-03 17:43:08 PDT
Created attachment 263678 [details]
Branch log from 2.0.0.4 RC1
Comment 6 Samuel Sidler (old account; do not CC) 2007-05-03 17:45:36 PDT
Created attachment 263679 [details]
Mac Trunk log from 2007-04-28

Based on the Mac branch crash log above, I don't think this is exploitable on the branch, but based on this trunk log, it could be exploitable there.

I'm also crashing in a completely different place than Martijn. Not sure if that's just Mac vs Windows. (And the branch and trunk crashes happen in different places as well but that's partially expected.)
Comment 7 Samuel Sidler (old account; do not CC) 2007-05-03 18:35:26 PDT
Boris, can you take a look at this? Or who's the best person to look at it?
Comment 8 Samuel Sidler (old account; do not CC) 2007-05-03 18:42:08 PDT
Created attachment 263681 [details]
Mac Branch Debug log from 1.8 branch (~2007050317)

This log from a debug build suggests this might be exploitable on the 1.8 branch, so I retract my above statement.
Comment 9 Boris Zbarsky [:bz] 2007-05-04 00:48:34 PDT
I won't be able to look at this on a mac for a few weeks, probably...  Not sure who's best to look, to be honest.  :(
Comment 10 Olli Pettay [:smaug] (vacation Aug 25-28) 2007-05-04 01:09:43 PDT
This is not a mac-only crash. (but non-debug-only maybe)
I'm really not familiar with stylecontext's memory handling, who owns what and how long. What should make sure that mRuleNode isn't deleted before stylecontext? dbaron?

#0  0x00000000006d0cf0 in ?? ()
#1  0x00002aaab0c182d6 in nsStyleContext::Destroy () from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libgklayout.so
#2  0x00002aaab0e0a4e8 in nsElementSH::PostCreate () from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libgklayout.so
#3  0x00002aaaaf519a30 in XPCWrappedNative::GetNewOrUsed ()
   from /home/smaug/mozilla/mozilla_cvs/mozilla/ff_build_non_debug/dist/bin/components/libxpconnect.so
Comment 11 Samuel Sidler (old account; do not CC) 2007-05-04 01:12:57 PDT
My crash log from comment 8 (attachment 263681 [details]) is from a branch debug build, so I don't think this is non-debug-only.
Comment 12 Boris Zbarsky [:bz] 2007-05-04 01:25:19 PDT
> I'm really not familiar with stylecontext's memory handling, who owns what
> and how long.

Style contexts are refcounted, but the memory is allocated from the presshell arena.  Which means if a style context outlives its presshell, it's toast.

Rule nodes are garbage collected, basically.  Every so often we nuke all the ones that are not pointed to by any style contexts.  See the NotifyStyleContextDestroyed() call in nsStyleContext::~nsStyleContext.  Again, they shouldn't be outliving the presshell.

Which brings us back to this bug.  Patch coming up, I think.
Comment 13 Boris Zbarsky [:bz] 2007-05-04 01:26:28 PDT
Created attachment 263698 [details] [diff] [review]
Does this fix the crash?

At a guess, the SVG node makes us not create frames for the <tabs>, so the binding is attached when we get the node, and the constructor fires the event, which kills off the presshell, etc.
Comment 14 Olli Pettay [:smaug] (vacation Aug 25-28) 2007-05-04 01:34:11 PDT
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

That fixes it. and the patch makes sense to me :)
Comment 15 Boris Zbarsky [:bz] 2007-05-04 01:39:36 PDT
Created attachment 263699 [details] [diff] [review]
Same as diff -w for ease of review
Comment 16 Boris Zbarsky [:bz] 2007-05-04 22:45:38 PDT
Fixed on trunk.
Comment 17 Boris Zbarsky [:bz] 2007-05-04 22:46:10 PDT
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

It's worth doing this on branches.  It's a very safe fix.
Comment 18 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-05-06 02:46:32 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070505 Minefield/3.0a5pre
Comment 19 Daniel Veditz [:dveditz] 2007-06-15 10:34:21 PDT
Comment on attachment 263698 [details] [diff] [review]
Does this fix the crash?

approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Comment 20 Boris Zbarsky [:bz] 2007-06-15 11:23:53 PDT
Fixed on both branches.
Comment 21 Carsten Book [:Tomcat] - PTO-back Sept 4th 2007-07-04 13:23:23 PDT
verified fixed 1.8.1.5 using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.5pre) Gecko/2007070403 BonEcho/2.0.0.5pre on Fedora F7 and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.5pre) Gecko/20070704 BonEcho/2.0.0.5pre ID:2007070403 - no crash with the testcase from this bug - adding verified keyword
Comment 22 juan becerra [:juanb] 2007-08-22 17:07:38 PDT
I wasn't able to reproduce the crash on Thunderbird 15012 or 15013 using the testcase #1. 
Comment 23 Carsten Book [:Tomcat] - PTO-back Sept 4th 2007-08-23 07:56:25 PDT
no crash on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.13pre) Gecko/20070822 Firefox/1.5.0.13pre with the testcase provided in this bug. Adding verified keyword for 1.8.0.13
Comment 24 Bob Clary [:bc:] 2009-04-24 10:51:56 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/3a53dc42292c

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