Closed Bug 503196 Opened 11 years ago Closed 11 years ago

[Mac] Crash in [@ nsBaseWidget::Destroy() ] while printing

Categories

(Core :: Widget: Cocoa, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1
Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed

People

(Reporter: marcia, Assigned: smichaud)

References

()

Details

(6 keywords, Whiteboard: [sg:moderate] critical if web site can cause crash w/out printing[crashkill][crashkill-fix])

Crash Data

Attachments

(3 files, 2 obsolete files)

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.
Keywords: crash
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.
Attached file 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..
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
Flags: wanted1.9.2?
Flags: wanted1.9.1.x?
Flags: blocking1.9.2?
Flags: blocking1.9.1.1?
Keywords: regression, testcase
Version: 1.9.1 Branch → Trunk
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.
Assignee: nobody → smichaud
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.
Component: Widget: Mac → Widget: Cocoa
QA Contact: mac → cocoa
Flags: wanted1.9.1.x? → wanted1.9.1.x+
blocking1.9.1: --- → needed
Flags: blocking1.9.1.1? → blocking1.9.1.1-
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.
Attached patch FixSplinter Review
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.
Attachment #388803 - Attachment is obsolete: true
Attachment #389127 - Flags: review?(joshmoz)
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.
Blocks: 487393
Flags: wanted1.9.0.x-
Whiteboard: [sg:moderate] critical if web site can cause crash w/out printing
Group: core-security
Comment on attachment 389127 [details] [diff] [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.
Attachment #389127 - Flags: review?(joshmoz) → review+
Landed on trunk (mozilla-central) with Josh's change:
http://hg.mozilla.org/mozilla-central/rev/c88543bb3635
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #389487 - Flags: approval1.9.1.2?
This is the #7 topcrasher on OS X (on the 1.9.1 branch and the trunk)
-- 486 instances in the last week.
Keywords: topcrash
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Flags: blocking1.9.2+
Comment on attachment 389487 [details] [diff] [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.
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.
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 on attachment 389487 [details] [diff] [review]
Fix for 1.9.1 branch

Let's take it in 1.9.1.3.
Attachment #389487 - Flags: approval1.9.1.2? → approval1.9.1.3?
Flags: wanted1.9.1.x+
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)
Keywords: fixed1.9.2
This is definitely fixed in today's Namoroka nightly.
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.
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.
1.9.1.3 has already been cut, from what I see.
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 on attachment 389487 [details] [diff] [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.
Attachment #389487 - Flags: approval1.9.1.3? → approval1.9.1.4?
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
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Target Milestone: --- → mozilla1.9.2a1
blocking1.9.1: needed → .4+
Comment on attachment 389487 [details] [diff] [review]
Fix for 1.9.1 branch

Approved for 1.9.1.4, a=dveditz for release-drivers
Attachment #389487 - Flags: approval1.9.1.4? → approval1.9.1.4+
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).
Keywords: verified1.9.1
Whiteboard: [sg:moderate] critical if web site can cause crash w/out printing → [sg:moderate] critical if web site can cause crash w/out printing[crashkill]
Whiteboard: [sg:moderate] critical if web site can cause crash w/out printing[crashkill] → [sg:moderate] critical if web site can cause crash w/out printing[crashkill][crashkill-fix]
Group: core-security
Crash Signature: [@ nsBaseWidget::Destroy() ]
You need to log in before you can comment on or make changes to this bug.