Last Comment Bug 279505 - Crash in pop-up window on parent.close() due to double free. [@ nsCSSFrameConstructor::RestyleEvent::HandleEvent]
: Crash in pop-up window on parent.close() due to double free. [@ nsCSSFrameCon...
Status: RESOLVED FIXED
: crash, fixed1.8.0.15, testcase, verified1.8.1.12
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.9beta3
Assigned To: Mats Palmgren (:mats)
: Hixie (not reading bugmail)
Mentors:
: 412693 413009 (view as bug list)
Depends on: 413587
Blocks: 412701
  Show dependency treegraph
 
Reported: 2005-01-23 13:15 PST by zanbabban
Modified: 2008-03-21 23:36 PDT (History)
24 users (show)
dveditz: blocking1.8.1.12+
dveditz: wanted1.8.1.x+
asac: blocking1.8.0.next+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Crash log. (3.58 KB, text/plain)
2005-01-23 13:22 PST, Wevah
no flags Details
Crash stack (Linux debug build) (8.67 KB, text/plain)
2005-01-23 21:19 PST, Mats Palmgren (:mats)
no flags Details
stacks (25.59 KB, text/html)
2008-01-16 17:35 PST, Mats Palmgren (:mats)
no flags Details
wallpaper (2.40 KB, patch)
2008-01-16 17:39 PST, Mats Palmgren (:mats)
no flags Details | Diff | Review
Branch patch, rev. 1 (3.61 KB, patch)
2008-01-18 02:29 PST, Mats Palmgren (:mats)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.1.12+
asac: approval1.8.0.next+
Details | Diff | Review
Trunk patch, rev. 1 (1.34 KB, patch)
2008-01-18 02:30 PST, Mats Palmgren (:mats)
bzbarsky: review+
bzbarsky: superreview+
mtschrep: approval1.9+
Details | Diff | Review
1.8.0 backport (2.86 KB, patch)
2008-03-21 23:35 PDT, Reed Loden [:reed] (use needinfo?)
no flags Details | Diff | Review

Description zanbabban 2005-01-23 13:15:37 PST
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050122 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050122 Camino/0.8+

Pop-up window has nested body tags.  Inner tag has an onLoad handler set to
parent.close().  When this fires, a double free is reported followed by a segfault.


Reproducible: Always

Steps to Reproduce:
1.  Visit URL shown above.
2.  Click link to popup window.
3.  When it loads, browser crashes.

Actual Results:  
Double free log followed immediately by segfault.

Expected Results:  
Just close the pop-up window.

Here's the log message from the console:
  *** malloc[26370]: error for object 0x606a4d0: Double free
  Segmentation fault

Camino nightly from 2005-01-19 was not affected.
Camino nightly from 2005-01-20 crashes.

2005-01-21 Firefox Mac nightly also crashes (from
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2005-01-21-07-trunk/firefox-1.0+.en-US.mac.dmg.gz
)

Note, removing the nested body tags (so that there's just a single body tag with
parent.close()), seems to avoid bug.  This is a test case that I derived from a
crash in the login dialog that is part of the popular PHP-based photo gallery
software distributed at gallery.menalto.com.

I don't know what component this would be, but someone else grabbed a stack
trace and indicated this crashed in the gecko code, so setting product to core.
Comment 1 zanbabban 2005-01-23 13:19:20 PST
Set component to DOM: Events due to onLoad. (Not sure this is right.)  
Comment 2 Wevah 2005-01-23 13:22:49 PST
Created attachment 172196 [details]
Crash log.

(Munged because I'm running OS X 10.2.)
Comment 3 Chris Applegate 2005-01-23 19:04:11 PST
I also get a crash on Linux. Talkback ID is TB3249472W and my version is
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8b) Gecko/20050120 Firefox/1.0+.
Clicking just once didn't cause the crash; I had to do it several times.
Comment 4 Mats Palmgren (:mats) 2005-01-23 21:19:46 PST
Created attachment 172229 [details]
Crash stack (Linux debug build)
Comment 5 Boris Zbarsky [:bz] 2005-01-23 21:51:36 PST
Hmm... I can't seem to reproduce this.  Are people seeing this in builds in
which bug 279205 is fixed?
Comment 6 Mats Palmgren (:mats) 2005-01-23 22:12:35 PST
Boris, I can crash a debug build with bug 279205 fixed. I have to click
the link quite fast, before the previous popup have disappeared.
Comment 7 Boris Zbarsky [:bz] 2005-01-23 22:50:25 PST
Oh, ok.  If I click the link at least 3 times in a row before the first window
comes up, I see the crash.

Dan, the frame constructor is getting events that it explicitly revoked... 
They're being dispatched when ~nsEventQueueStack in
nsXULWindow::CreateNewContentWindow pops the thread event queue.  If I click
_more_ than three times, I actually end up with multiple nested calls to
nsXULWindow::CreateNewContentWindow on the stack.

dmose suggested that maybe there is a locking issue in revokeEvents.  Note also
that David recently ran into a similar problem (see
http://lxr.mozilla.org/seamonkey/source/view/src/nsViewManager.cpp#361).

I don't see anything obviously wrong in revokeEvents or the NSPR code it calls,
but I don't quite follow the intricacies of all that code...
Comment 8 Charles Fenwick 2005-03-29 08:49:12 PST
Despite my clicking madness (4 clicks before first window comes up), am unable
to reproduce on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050328.
Comment 9 Mats Palmgren (:mats) 2005-03-29 16:07:06 PST
I can't reproduce the crash either (in a debug build). But I do see:

WARNING: event queue chain length is 6. this is almost certainly a leak., file
nsEventQueue.cpp, line 549

so we should keep this bug open for further analysis, see comment 7.
Comment 10 Boris Zbarsky [:bz] 2005-03-29 16:29:38 PST
Yeah, I can't reproduce the crash either, anymore... I'd file a separate bug on
the event queue length issue (that's caused by new window.open calls before the
previous window has finished opening).

Note also bug 284389, though...  That could still cause crashes here in some
circumstances, I bet.
Comment 11 chris hofmann 2006-03-14 17:24:06 PST
should we close this out now.  bug 284389 is fixed, test url no longer available...
Comment 12 Wayne Mery (:wsmwk, NI for questions) 2006-03-29 09:35:33 PST
don't know that this is the same problem, or that it's more than a fluke (as I've never had FF crash like this clicking a link)

TB16957942

Stack Signature	 nsCSSFrameConstructor::RestyleEvent::HandleEvent 7d740223
Product ID	Firefox15
Build ID	2005111116
Trigger Time	2006-03-28 08:42:01.0
Platform	Win32
Operating System	Windows NT 5.0 build 2195
Module	firefox.exe + (00186106)
URL visited	http://www.dft.nl
User Comments	right click followed by copy URL location (in Bug 39573) s/vseerror
Since Last Crash	2795954 sec
Total Uptime	8198774 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13907
Stack Trace 	
nsCSSFrameConstructor::RestyleEvent::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13907]
SETUPAPI.DLL + 0x30c24 (0x778b0c24)
Comment 13 Henrik Skupin (:whimboo) 2008-01-16 11:59:26 PST
I exactly get this crash with the latest Thunderbird 2.0.0.12 nightly build and the extension "Minimize To Tray" installed. Yesterday I hadn't any problem with this fresh combination but today it always crashes while trying to read a specific message with attachment. Is it the same like this bug or should I file a new bug? The stack only consists of 3 frames...

Stack Signature	 nsCSSFrameConstructor::RestyleEvent::HandleEvent c30de5df
Product ID	Thunderbird2
Build ID	2008011503
Trigger Time	2008-01-16 06:43:28.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	thunderbird.exe + (0022acc2)
URL visited	
User Comments	Edit recipient while forwarding a message crashes Thunderbird
Since Last Crash	53 sec
Total Uptime	53 sec
Trigger Reason	Access violation
Source File, Line No.	e:/builds/tinderbox/Tb-Mozilla1.8/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 14256
Stack Trace 	
nsCSSFrameConstructor::RestyleEvent::HandleEvent  [mozilla/layout/base/nsCSSFrameConstructor.cpp, line 14256]
HandleRestyleEvent  [mozilla/layout/base/nsCSSFrameConstructor.cpp, line 14281]
0x778b0c24
nsScriptLoader::ProcessRequest  [mozilla/content/base/src/nsScriptLoader.cpp, line 694]
0x8a084689
Comment 14 Unknown W. Brackets 2008-01-16 13:22:00 PST
I'm also getting this (it looks like) very frequently with both latest branch of Thunderbird and Firefox.

For Firefox, it happens whenever I have a wysiwyg editor on a page and close that tab, then open another tab with another wysiwyg editor.

For Thunderbird, happens when shutting down the program (no where near as annoying, assuming my data is safe.)

TB40260872K
TB40286768K
TB40286699E
TB40262025W

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12pre) Gecko/20080116 BonEcho/2.0.0.12pre

Thunderbird version 2.0.0.12pre (20080115)

-[Unknown]
Comment 15 Mats Palmgren (:mats) 2008-01-16 15:15:33 PST
Do you know the build ID of the build before that didn't crash so we have
an exact regression range?  If you don't know, maybe you can try a few
builds from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/
(folders ending in "-mozilla1.8/"), that would help a lot. Thanks.
Comment 16 Mats Palmgren (:mats) 2008-01-16 15:22:34 PST
Guessing a tentative regression range 2008011403 -- 2008011503:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-01-14+02%3A00&maxdate=2008-01-15+04%3A00&cvsroot=%2Fcvsroot

Boris, bug 396613 seems the most likely culprit in that range.

This should block 2.0.0.12 until we can explain the sudden regressions.
Comment 17 Unknown W. Brackets 2008-01-16 15:26:48 PST
(In reply to comment #16)
> Guessing a tentative regression range 2008011403 -- 2008011503:
> http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-01-14+02%3A00&maxdate=2008-01-15+04%3A00&cvsroot=%2Fcvsroot
> 
> Boris, bug 396613 seems the most likely culprit in that range.
> 
> This should block 2.0.0.12 until we can explain the sudden regressions.
> 

I use Thunderbird on a daily basis (and update nightlies on a daily basis), as do I deal with content management systems which use midas/etc. nearly every day.  I can confirm that that is the regression range - at least for my problem.

-[Unknown]
Comment 18 Boris Zbarsky [:bz] 2008-01-16 15:28:05 PST
Sure.  That bug could cause more restyle events.  The basic problem remains what I said in comment 7, as far as I could tell at the time...
Comment 19 Mats Palmgren (:mats) 2008-01-16 17:35:26 PST
Created attachment 297469 [details]
stacks

Editor calls ReconstructStyleData() on a shell that has been Destroy()'ed.
Comment 20 Mats Palmgren (:mats) 2008-01-16 17:39:18 PST
Created attachment 297470 [details] [diff] [review]
wallpaper
Comment 21 Boris Zbarsky [:bz] 2008-01-16 19:20:13 PST
Yeah, I was going to ask.  That does seem like the right thing to do.  Does trunk need that too?
Comment 22 Boris Zbarsky [:bz] 2008-01-16 19:24:26 PST
*** Bug 412701 has been marked as a duplicate of this bug. ***
Comment 23 Henrik Skupin (:whimboo) 2008-01-17 01:36:40 PST
This bug is really nasty for my Thunderbird 2 branch build. It crashes or doesn't correctly respond to any events afterwards I've sent a special message. I could click e.g. on Tools - Error Console and nothing happens for a while. After a minute or so the window is shown. Same applies to any context menu or other windows.
Comment 24 Henrik Skupin (:whimboo) 2008-01-17 01:48:45 PST
(In reply to comment #23)
> After a minute or so the window is shown. Same applies to any context menu or

That wasn't correct. The Error Console window and each other one is shown after I close the main window. Seems that the events hanging around and aren't processed until the target is closed.

Comment 25 Mats Palmgren (:mats) 2008-01-17 15:46:14 PST
(In reply to comment #21)
> Does trunk need that too?

It was already fixed on trunk in bug 315453.  However, I still found one
trunk crash that looks suspicious: bp-3877f0ba-c2d9-11dc-8592-001a4bd43ef6
PostRestyleEvent() is called in just a few more places:
http://lxr.mozilla.org/seamonkey/search?string=PostRestyleEvent
mostly from ContentStatesChanged/AttributeChanged methods, but those
are called from several hundred places...

nsCSSFrameConstructor has 'mIsDestroyingFrameTree' that basically signals
that the shell is destroyed, I think it might be worth adding an early
return in PostRestyleEvent() if it's true - at least on branch where we
have less mileage on doing async restyle events.
Comment 26 Boris Zbarsky [:bz] 2008-01-17 18:32:39 PST
Doing an early return in PostRestyleEvent if  'mIsDestroyingFrameTree' makes total sense.  Let's do that!
Comment 27 Mats Palmgren (:mats) 2008-01-18 02:29:03 PST
Created attachment 297724 [details] [diff] [review]
Branch patch, rev. 1
Comment 28 Mats Palmgren (:mats) 2008-01-18 02:30:35 PST
Created attachment 297725 [details] [diff] [review]
Trunk patch, rev. 1

I added the assertion because I think it's worth investigating should
it occur.  Mochitest and reftest suites on Linux didn't trigger it.
Comment 29 Mats Palmgren (:mats) 2008-01-18 10:51:59 PST
MOZILLA_1_8_BRANCH:
mozilla/layout/base/nsPresShell.cpp     3.852.2.30
mozilla/layout/base/nsCSSFrameConstructor.cpp   1.1110.6.93 

Trunk:
mozilla/layout/base/nsCSSFrameConstructor.cpp   1.1455 

FWIW, I tried to make a testcase from the description in comment 0 but
without success.  (URL is 404 and I didn't have a local copy :-( )

-> FIXED
Comment 30 Daniel Veditz [:dveditz] 2008-01-18 11:50:40 PST
This was checked in on branch without approval.

Trunk and branch rules are different. "blocking" might be good enough on trunk which is in a beta development period but the branch is essentially in final release-candidate lockdown all the time. Explicit approval is always required.
Comment 31 Mats Palmgren (:mats) 2008-01-18 12:35:29 PST
Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

Oops, sorry.
Comment 32 Al Billings [:abillings] 2008-01-18 13:33:56 PST
I get a 403 permission error when I try to go to http://aoeu.mine.nu/~zan/crash.html to verify this fix. 

I need a test case or working url to verify this fix in branch for the release.
Comment 33 Samuel Sidler (old account; do not CC) 2008-01-18 13:43:10 PST
(In reply to comment #32)
> I need a test case or working url to verify this fix in branch for the release.

You can verify this with Thunderbird branch. See bug 412701 for steps. Marcia, Tomcat, or stephend can also verify it.
Comment 34 Stuart Morgan 2008-01-18 17:36:46 PST
*** Bug 412693 has been marked as a duplicate of this bug. ***
Comment 35 Stuart Morgan 2008-01-18 19:16:31 PST
*** Bug 413009 has been marked as a duplicate of this bug. ***
Comment 36 Daniel Veditz [:dveditz] 2008-01-22 13:51:33 PST
Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

approved for 1.8.1.12, a=dveditz for release-drivers
Comment 37 Jesse Ruderman 2008-01-22 19:26:38 PST
The assertion has been spotted: bug 413587.
Comment 38 Stephen Donner [:stephend] - PTO; back on 5/28 2008-01-28 21:02:32 PST
I could reproduce this 100% with earlier Thunderbird branch builds, but can longer, with:

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12pre) Gecko/2008012804 Thunderbird/2.0.0.12pre.

Also, from the duped bug 412693 (which has a separate testcase), the reporter there reports that the update fixed it for him, too.

Verified FIXED, so replacing fixed1.8.1.12 keyword with verified1.8.1.12.
Comment 39 Alexander Sack 2008-03-18 05:28:31 PDT
Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

a=asac for 1.8.0.15
Comment 40 Henrik Skupin (:whimboo) 2008-03-19 16:23:54 PDT
Any chance to get a testcase for this bug? The given URL isn't available anymore. I would like to verify this on trunk.
Comment 41 Reed Loden [:reed] (use needinfo?) 2008-03-21 23:35:18 PDT
Created attachment 311113 [details] [diff] [review]
1.8.0 backport
Comment 42 Reed Loden [:reed] (use needinfo?) 2008-03-21 23:36:56 PDT
MOZILLA_1_8_0_BRANCH:

Checking in layout/base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1110.6.12.2.69; previous revision: 1.1110.6.12.2.68
done
Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.852.2.11.2.14; previous revision: 3.852.2.11.2.13
done

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