Closed
Bug 503196
Opened 15 years ago
Closed 15 years ago
[Mac] Crash in [@ nsBaseWidget::Destroy() ] while printing
Categories
(Core :: Widget: Cocoa, defect)
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)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
jaas
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dveditz
:
approval1.9.1.4+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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
Assignee | ||
Comment 4•15 years ago
|
||
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
Updated•15 years ago
|
Flags: wanted1.9.1.x? → wanted1.9.1.x+
Updated•15 years ago
|
Assignee | ||
Comment 6•15 years ago
|
||
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.
Assignee | ||
Comment 7•15 years ago
|
||
Attachment #388799 -
Attachment is obsolete: true
Assignee | ||
Comment 8•15 years ago
|
||
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)
Assignee | ||
Comment 9•15 years ago
|
||
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•15 years ago
|
||
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
Updated•15 years ago
|
Group: core-security
Comment 11•15 years ago
|
||
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+
Assignee | ||
Comment 12•15 years ago
|
||
Landed on trunk (mozilla-central) with Josh's change:
http://hg.mozilla.org/mozilla-central/rev/c88543bb3635
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•15 years ago
|
||
Assignee | ||
Updated•15 years ago
|
Attachment #389487 -
Flags: approval1.9.1.2?
Assignee | ||
Comment 14•15 years ago
|
||
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
Updated•15 years ago
|
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Flags: blocking1.9.2+
Comment 15•15 years ago
|
||
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.
Assignee | ||
Comment 16•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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?
Updated•15 years ago
|
Flags: wanted1.9.1.x+
Comment 19•15 years ago
|
||
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
Assignee | ||
Comment 20•15 years ago
|
||
This is definitely fixed in today's Namoroka nightly.
Comment 21•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
1.9.1.3 has already been cut, from what I see.
Comment 24•15 years ago
|
||
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•15 years ago
|
||
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?
Assignee | ||
Comment 26•15 years ago
|
||
This is also a topcrasher in FF:
http://crash-stats.mozilla.com/report/list?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
(3145 crashes in the last week, #4 on OS X)
http://crash-stats.mozilla.com/report/list?platform=mac&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsChildView%3A%3ADestroy%28%29
(698 crashes in the last week, #27 on OS X)
Comment 27•15 years ago
|
||
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?
Keywords: fixed1.9.2 → verified1.9.2
Target Milestone: --- → mozilla1.9.2a1
Updated•15 years ago
|
blocking1.9.1: needed → .4+
Comment 28•15 years ago
|
||
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+
Assignee | ||
Comment 29•15 years ago
|
||
Landed on the 1.9.1 branch:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/02b97cf4609f
Updated•15 years ago
|
status1.9.2:
--- → beta1-fixed
Comment 30•15 years ago
|
||
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
Updated•15 years ago
|
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]
Updated•15 years ago
|
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]
Updated•15 years ago
|
Group: core-security
Updated•13 years ago
|
Crash Signature: [@ nsBaseWidget::Destroy() ]
You need to log in
before you can comment on or make changes to this bug.
Description
•