Closed Bug 503196 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: Widget: Cocoa, defect)

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 (deleted) —
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.
Attachment #388799 - Attachment is obsolete: true
Attached patch Fix (deleted) — Splinter 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: 15 years ago
Resolution: --- → FIXED
Attached patch Fix for 1.9.1 branch (deleted) — Splinter Review
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.

Attachment

General

Created:
Updated:
Size: