zanbabban Reporter
	Description • 20 years ago

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.

	zanbabban Reporter
	Comment 1 • 20 years ago

Set component to DOM: Events due to onLoad. (Not sure this is right.)  

	Wevah (Nate Weaver)
	Comment 2 • 20 years ago

(Munged because I'm running OS X 10.2.)

	Chris Applegate
	Comment 3 • 20 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 20 years ago

Hmm... I can't seem to reproduce this.  Are people seeing this in builds in
which bug 279205 is fixed?

	Mats Palmgren (inactive) Assignee
	Comment 6 • 20 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 20 years ago

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...

	Charles Fenwick
	Comment 8 • 20 years ago

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.

	Mats Palmgren (inactive) Assignee
	Comment 9 • 20 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 20 years ago

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.

	chris hofmann
	Comment 11 • 19 years ago

should we close this out now.  bug 284389 is fixed, test url no longer available...

	Wayne Mery (:wsmwk)
	Comment 12 • 19 years ago

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)

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 13 • 17 years ago

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

	Unknown W. Brackets
	Comment 14 • 17 years ago

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]

	Mats Palmgren (inactive) Assignee
	Comment 15 • 17 years ago

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.

	Mats Palmgren (inactive) Assignee
	Comment 16 • 17 years ago

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.

	Unknown W. Brackets
	Comment 17 • 17 years ago

(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]

	Boris Zbarsky [:bzbarsky]
	Comment 18 • 17 years ago

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...

	Mats Palmgren (inactive) Assignee
	Comment 19 • 17 years ago

Editor calls ReconstructStyleData() on a shell that has been Destroy()'ed.

	Boris Zbarsky [:bzbarsky]
	Comment 21 • 17 years ago

Yeah, I was going to ask.  That does seem like the right thing to do.  Does trunk need that too?

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 23 • 17 years ago

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.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 24 • 17 years ago

(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.

	Mats Palmgren (inactive) Assignee
	Comment 25 • 17 years ago

(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.

	Boris Zbarsky [:bzbarsky]
	Comment 26 • 17 years ago

Doing an early return in PostRestyleEvent if  'mIsDestroyingFrameTree' makes total sense.  Let's do that!

	Mats Palmgren (inactive) Assignee
	Comment 28 • 17 years ago

I added the assertion because I think it's worth investigating should
it occur.  Mochitest and reftest suites on Linux didn't trigger it.

	Mats Palmgren (inactive) Assignee
	Comment 29 • 17 years ago

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

	Daniel Veditz [:dveditz]
	Comment 30 • 17 years ago

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.

	Mats Palmgren (inactive) Assignee
	Comment 31 • 17 years ago

Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

Oops, sorry.

	Al Billings [:abillings - ex-MoCo]
	Comment 32 • 17 years ago

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.

	Samuel Sidler (old account; do not CC)
	Comment 33 • 17 years ago

(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.

	Daniel Veditz [:dveditz]
	Comment 36 • 17 years ago

Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

approved for 1.8.1.12, a=dveditz for release-drivers

	Jesse Ruderman
	Comment 37 • 17 years ago

The assertion has been spotted: bug 413587.

	Stephen Donner [:stephend] Not actively reading bugmail
	Comment 38 • 17 years ago

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.

	Alexander Sack
	Comment 39 • 17 years ago

Comment on attachment 297724 [details] [diff] [review]
Branch patch, rev. 1

a=asac for 1.8.0.15

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 40 • 17 years ago

Any chance to get a testcase for this bug? The given URL isn't available anymore. I would like to verify this on trunk.

	Reed Loden [:reed]
	Comment 42 • 17 years ago

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