Last Comment Bug 503196 - [Mac] Crash in [@ nsBaseWidget::Destroy() ] while printing
: [Mac] Crash in [@ nsBaseWidget::Destroy() ] while printing
Status: VERIFIED FIXED
: [sg:moderate] critical if web site ca...
: crash, regression, testcase, topcrash, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Widget: Cocoa
: Trunk
: x86 Mac OS X
: -- critical (vote)
: mozilla1.9.2a1
Assigned To: Steven Michaud
: cocoa
: http://www.dierenartspeeters.be/nl/co...
:
: 487393
  Show dependency treegraph
 
Reported: 2009-07-08 15:55 PDT by Marcia Knous [:marcia]
Modified: 2009-11-09 18:52 PST (History)
17 users (show)
benjamin: blocking1.9.2+
mbeltzner: blocking1.9.1.1-
dveditz: wanted1.9.0.x-
hskupin: in‑testsuite?
See Also:
Crash Signature:
[@ nsBaseWidget::Destroy() ]
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  beta1-fixed
  .4+
  .4-fixed


Attachments
testcase (331 bytes, text/html)
2009-07-09 01:14 PDT, Martijn Wargers [:mw22] (QA - IRC nick: mw22)
no flags Details
Debugging patch (partially disable patch for bug 487393), *not* a fix (1.88 KB, patch)
2009-07-15 15:10 PDT, Steven Michaud
no flags Details | Diff | Splinter Review
Debugging patch rev1 (minimize further), still *not* a fix (780 bytes, patch)
2009-07-15 15:19 PDT, Steven Michaud
no flags Details | Diff | Splinter Review
Fix (2.96 KB, patch)
2009-07-17 08:33 PDT, Steven Michaud
joshmoz: review+
Details | Diff | Splinter Review
Fix for 1.9.1 branch (3.57 KB, patch)
2009-07-20 08:15 PDT, Steven Michaud
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Summon comment box

Description Marcia Knous [:marcia] 2009-07-08 15:55:23 PDT
Seen while running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

STR:
1. Visit the site in the URL.
2. Print to a local printer - in my case an Epson Stylus C88+
3. Crash immediately, 100% reproducible.

Breakpad: http://crash-stats.mozilla.com/report/index/2deeca2f-b2fe-4055-8468-2ad792090708. There are currently 576 crashes in this stack for people using Mac and Firefox 3.5, and several of the comments reference printing. I obtained the URL from the crash comments.
Comment 1 Marcia Knous [:marcia] 2009-07-08 17:01:50 PDT
mw22 asked me to try printing this site from a Windows machine to see if I got the crash as well, but I do not get the crash printing from a Win Vista machine.
Comment 2 Martijn Wargers [:mw22] (QA - IRC nick: mw22) 2009-07-09 01:14:25 PDT
Created attachment 387608 [details]
testcase

Thanks, Marcia. That means this seems to be a Mac widget issue.

The testcase crashes with this stacktrace:
http://crash-stats.mozilla.com/report/index/262f155e-9b16-4ea0-ac28-431c72090709?p=1
0  	XUL  	nsBaseWidget::Destroy  	 widget/src/xpwidgets/nsBaseWidget.cpp:252
1 	XUL 	nsChildView::Destroy 	widget/src/cocoa/nsChildView.mm:718
2 	XUL 	XUL@0x569f4f 	
3 	XUL 	XUL@0x569f3f 	
4 	XUL 	nsFrame::Destroy 	layout/generic/nsFrame.cpp:483
5 	XUL 	nsLineBox::DeleteLineList 	layout/generic/nsLineBox.cpp:338
6 	XUL 	nsBlockFrame::Destroy 	layout/generic/nsBlockFrame.cpp:298
7 	XUL 	nsLineBox::DeleteLineList 	layout/generic/nsLineBox.cpp:338
8 	XUL 	nsBlockFrame::Destroy 	layout/generic/nsBlockFrame.cpp:298
9 	XUL 	XUL@0x26d61c 	
10 	XUL 	nsContainerFrame::Destroy 	layout/generic/nsContainerFrame.cpp:266
11 	XUL 	CanvasFrame::Destroy 	layout/generic/nsHTMLFrame.cpp:227
12 	XUL 	XUL@0x26d61c 	
13 	XUL 	nsContainerFrame::Destroy 	layout/generic/nsContainerFrame.cpp:266
14 	XUL 	XUL@0x26d61c 	
15 	XUL 	nsContainerFrame::Destroy 	layout/generic/nsContainerFrame.cpp:266
16 	XUL 	nsFrameManager::Destroy 	layout/base/nsFrameManager.cpp:290
17 	XUL 	PresShell::Destroy 	layout/base/nsPresShell.cpp:2035
18 	XUL 	nsPrintObject::DestroyPresentation 	layout/printing/nsPrintObject.cpp:98
19 	XUL 	nsPrintEngine::SetupToPrintContent 	layout/printing/nsPrintEngine.cpp:1678
20 	XUL 	nsPrintEngine::DoCommonPrint 	
etc..
Comment 3 Martijn Wargers [:mw22] (QA - IRC nick: mw22) 2009-07-09 01:21:53 PDT
The testcase doesn't crash Firefox 3, so this is a regression. It also crashes trunk. A regression range might be useful (I'm afraid I don't have the builds to do that for the Mac).

This is nr. 7 in the topcrash list for Firefox 3.5 on the Mac:
http://crash-stats.mozilla.com/query/query?do_query=1&product=Firefox&version=Firefox%3A3.5&platform=mac&date=&range_value=1&range_unit=weeks&query_search=signature&query_type=exact&query=
Basically all the comments mentioned that they were trying to print something:
http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.5&platform=mac&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsBaseWidget%3A%3ADestroy%28%29
Comment 4 Steven Michaud 2009-07-09 08:09:05 PDT
I can't reproduce the crash with this bug's URL
(http://www.dierenartspeeters.be/nl/coordinaten-15.htm, which may have
changed).  But I can easily reproduce it with Martijn's testcase
(attachment 387608 [details]), using File : Print : Preview.

The crash is almost certainly caused by accessing a deleted object (at
the call to parent->RemoveChild(this) in nsBaseWidget::Destroy(),
'parent' has already been deleted).

The two Breakpad reports linked here both pick up bug 470487 as
related.  This seems to be true (even though these two bugs aren't
dups, and don't have the same trigger).

I'll be working on this.
Comment 5 Smokey Ardisson (offline for a while; no bugmail - do not email) 2009-07-09 09:10:03 PDT
Not Widget:Mac if it's Firefox 3.5 ;)

(In reply to comment #4)
> The two Breakpad reports linked here both pick up bug 470487 as
> related.  This seems to be true (even though these two bugs aren't
> dups, and don't have the same trigger).

The logic there is that "related" bugs mention the report's signature somewhere in them; it's not very sophisticated, but it may find you bugs that actually *are* related.
Comment 6 Steven Michaud 2009-07-15 15:10:14 PDT
Created attachment 388799 [details] [review]
Debugging patch (partially disable patch for bug 487393), *not* a fix

Here are the regression ranges for this bug (on the trunk and the
1.9.1 branch):

firefox-2009-04-23-04-mozilla-central
firefox-2009-04-24-03-mozilla-central

firefox-2009-05-15-03-mozilla-1.9.1
firefox-2009-05-16-03-mozilla-1.9.1

This implicates my patch for bug 487393.

But there's nothing wrong with my patch.  Instead it seems to uncover
an existing bug.  This can be seen by the fact that my "debugging
patch", which reverses part of the patch for bug 487393, makes this
bug's crash stop happening.

I probably won't be able to work on this bug for at least the next
three weeks -- Josh wants me to work on other stuff, and I have a
week's vacation coming up.
Comment 7 Steven Michaud 2009-07-15 15:19:50 PDT
Created attachment 388803 [details] [review]
Debugging patch rev1 (minimize further), still *not* a fix
Comment 8 Steven Michaud 2009-07-17 08:33:20 PDT
Created attachment 389127 [details] [review]
Fix

Turns out this bug *was* caused by my patch for bug 487393.

With that patch, it now became possible for the list of a parent's
children to be changed while it was being iterated.  This meant that
if the parent had two children, only the first in the list would be
found.  Which in turn meant that the second child's reference to its
parent (mParentWidget) didn't get removed when the parent was deleted.
Which caused a crash when Destroy() was called on the second child,
and it tried to deference the second child's mParentWidget pointer.

This patch fixes the problem.

A tryserver build will follow in a few hours.
Comment 9 Steven Michaud 2009-07-17 10:21:52 PDT
Here's a tryserver build made with my patch from comment #8:
https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503196/bugzilla503196-macosx.dmg
Comment 10 Daniel Veditz 2009-07-19 12:07:04 PDT
This became a potential security vulnerability at comment 4 -- please don't hesitate to hide these bugs if you can or mail security@mozilla.org if you can't. We don't need more instances of the 3.5.1 firedrill where some joker uses our own testcases against us.
Comment 11 Josh Aas (Mozilla Corporation) 2009-07-19 20:15:16 PDT
Comment on attachment 389127 [details] [review]
Fix

// notify the children that we're gone

Please modify this comment to explain why you're iterating iterating this way. Modify the other similar comment in the patch too.
Comment 12 Steven Michaud 2009-07-20 08:05:21 PDT
Landed on trunk (mozilla-central) with Josh's change:
http://hg.mozilla.org/mozilla-central/rev/c88543bb3635
Comment 13 Steven Michaud 2009-07-20 08:15:34 PDT
Created attachment 389487 [details] [review]
Fix for 1.9.1 branch
Comment 14 Steven Michaud 2009-07-20 08:22:00 PDT
This is the #7 topcrasher on OS X (on the 1.9.1 branch and the trunk)
-- 486 instances in the last week.
Comment 15 Samuel Sidler (old account; do not CC) 2009-07-23 09:50:44 PDT
Comment on attachment 389487 [details] [review]
Fix for 1.9.1 branch

Steven: Given that we have no tests here, is there anything you can do to put our minds at ease for taking this patch? Are there other code paths that touch this code? How can we ensure this doesn't cause other crashes?

Let's revisit this after baking a bit.
Comment 16 Steven Michaud 2009-07-23 10:03:25 PDT
Frankly, I think this patch is trivial:  In my patch for bug 487393, I
forgot to ensure that I iterated the parents' children safely (so as
to allow the list to change while being iterated).  This was (blush) a
dumb mistake, which my current patch fixes.  I'm surprised Josh even
asked me to make a comment.
Comment 17 Josh Aas (Mozilla Corporation) 2009-07-23 11:21:02 PDT
This is not a high risk patch and in my opinion we should definitely take this by 1.9.1.3. As for 1.9.1.2 or 1.9.1.3 I don't have a strong opinion.
Comment 18 Samuel Sidler (old account; do not CC) 2009-07-23 11:27:32 PDT
Comment on attachment 389487 [details] [review]
Fix for 1.9.1 branch

Let's take it in 1.9.1.3.
Comment 19 Mike Beltzner [:beltzner] 2009-08-25 10:39:30 PDT
Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)
Comment 20 Steven Michaud 2009-08-25 11:02:03 PDT
This is definitely fixed in today's Namoroka nightly.
Comment 21 Robert Kaiser (:kairo@mozilla.com) 2009-08-26 05:50:38 PDT
Any chance to get this into 1.9.1.4? I was just informed that this seems to be one of the topcrashers on SeaMonkey.
Comment 22 Martijn Wargers [:mw22] (QA - IRC nick: mw22) 2009-08-26 06:05:25 PDT
Won't this get into the 1.9.1.3 release then? I think we've been waiting long enough now.
See comment 17, where it says we should be take it by 1.9.1.3.
Comment 23 Robert Kaiser (:kairo@mozilla.com) 2009-08-26 06:07:07 PDT
1.9.1.3 has already been cut, from what I see.
Comment 24 Martijn Wargers [:mw22] (QA - IRC nick: mw22) 2009-08-26 06:13:50 PDT
Ugh :(
How is that possible? I guess the blocking1.9.1 needed flag is not blocking anything at all? Doesn't it make that flag useless?
Comment 25 Henrik Skupin (:whimboo) 2009-08-26 06:27:17 PDT
Comment on attachment 389487 [details] [review]
Fix for 1.9.1 branch

Yes, too late for 1.9.1.3. Let's ask for approval to get it into 1.9.1.4.
Comment 27 Henrik Skupin (:whimboo) 2009-08-31 11:34:47 PDT
No crash reports on 1.9.2 for three weeks now. Looks like we can safely mark this bug as verified.

Also printing the testcase doesn't crash my Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a2pre) Gecko/20090825 Namoroka/3.6a2pre ID:20090825033555
Comment 28 Daniel Veditz 2009-08-31 15:32:18 PDT
Comment on attachment 389487 [details] [review]
Fix for 1.9.1 branch

Approved for 1.9.1.4, a=dveditz for release-drivers
Comment 29 Steven Michaud 2009-09-01 08:52:56 PDT
Landed on the 1.9.1 branch:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/02b97cf4609f
Comment 30 Al Billings [:abillings] 2009-09-14 15:58:43 PDT
Verified that Martijn's testcase still crashes 1.9.1.3 and does not crash 1.9.1.4 (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090914 Shiretoko/3.5.4pre).

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