Closed Bug 726206 Opened 12 years ago Closed 8 years ago

Firefox crash @ _dwrite_draw_glyphs_to_gdi_surface_gdi while printing

Categories

(Core :: Graphics, defect)

7 Branch
x86
Windows
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 + fixed
firefox48 + fixed
firefox49 --- fixed
firefox-esr45 - wontfix
relnote-firefox --- -

People

(Reporter: marcia, Assigned: lsalzman)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

Seen while looking at the Firefox 10 explosive report. http://tinyurl.com/88tk4cr to the crashes which are all Windows.

Comments:

I was using printPDF on the website.
Was using a plugin, PDFCreator to print a document when it crashed. Ended the plugin process which crashed Firefox, but the document finished printing.

https://crash-stats.mozilla.com/report/index/3d8c4c31-f39d-4b94-b456-f1a872120210

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	_dwrite_draw_glyphs_to_gdi_surface_gdi 	gfx/cairo/cairo/src/cairo-dwrite-font.cpp:1153
1 	xul.dll 	_cairo_dwrite_scaled_font_init_glyph_surface 	gfx/cairo/cairo/src/cairo-dwrite-font.cpp:1012
2 	xul.dll 	_cairo_dwrite_scaled_glyph_init 	gfx/cairo/cairo/src/cairo-dwrite-font.cpp:539
3 	xul.dll 	_cairo_scaled_glyph_lookup 	
4 		@0x6 	
5 	xul.dll 	_clip_and_composite 	gfx/cairo/cairo/src/cairo-image-surface.c:2321
6 	xul.dll 	_cairo_composite_rectangles_intersect 	gfx/cairo/cairo/src/cairo-composite-rectangles.c:98
7 	xul.dll 	_cairo_image_surface_glyphs 	gfx/cairo/cairo/src/cairo-image-surface.c:4080
8 	xul.dll 	_composite_glyphs 	gfx/cairo/cairo/src/cairo-image-surface.c:4026
Keywords: regression
Summary: Firefox crash @ _dwrite_draw_glyphs_to_gdi_surface_gdi(_cairo_win32_surface → Firefox crash @ _dwrite_draw_glyphs_to_gdi_surface_gdi while printing
Version: 10 Branch → 7 Branch
Reproduced the crash _once_ with the following steps:

1. Open http://mozqa.com/data/firefox/awesome_bar/diff.txt
2. Print the document
-> progress stops at 96% (see bug #762868)
3. Wait awhile and press "Cancel" in print progress window
4. Try to use the browser -> browser is freezed
5. Maximize and restore window

After step 5, Firefox crashed.
Got this crash in v33 after a black screen event. Very similar in symptoms to bug 903842 prior to the crash.
Sorry about the bug spam, but I should point out that I wasn't printing anything. Just scrolling around a normal web page, maximizing/restoring Firefox.
The report contains the error code corresponding to E_OUTOFMEMORY 
Looks like we failed to allocate the render target (because not enough memory was available) here: http://hg.mozilla.org/releases/mozilla-release/annotate/0cd2e9a8ba6f/gfx/cairo/cairo/src/cairo-dwrite-font.cpp#l1141 and the next instruction tries to dereference it without checking if creating the target failed.
I've had other crashes related to Out Of Memory. Oh well, one day the 64-bit build should hopefully resolve this once and for all, as I actually have plenty of RAM, and Firefox only runs out of its 32-bit address space.
(In reply to Roman from comment #6)
> I've had other crashes related to Out Of Memory. Oh well, one day the 64-bit
> build should hopefully resolve this once and for all, as I actually have
> plenty of RAM, and Firefox only runs out of its 32-bit address space.

Two solutions for now:
1) since Nightly 64-bit testing build for Windows is stable enough, you can use it;

2) or you can use https://addons.mozilla.org/firefox/addon/prevent-out-of-virtual-memo/ -- the most accurate way to prevent out of virtual memory crashes by automatically restarting FireFox once the limit is approached. Unlike regular extensions that track just size of FireFox, this extension tracks specifically virtual memory size, which allows to use restart more rarely.

I wish I could say "3) wait a little bit until FireFox 64-bit will be officially supported", but this things takes forever so it is not a viable option.
Crash Signature: [@ _dwrite_draw_glyphs_to_gdi_surface_gdi(_cairo_win32_surface*, DWRITE_MATRIX*, DWRITE_GLYPH_RUN*, unsigned long, _cairo_dwrite_scaled_font*, tagRECT const&) ] → [@ _dwrite_draw_glyphs_to_gdi_surface_gdi(_cairo_win32_surface*, DWRITE_MATRIX*, DWRITE_GLYPH_RUN*, unsigned long, _cairo_dwrite_scaled_font*, tagRECT const&) ] [@ _dwrite_draw_glyphs_to_gdi_surface_gdi ]
Assignee: nobody → jmuizelaar
I looked at one of the minidumps. We did not look to be printing. We failed allocating an 8x8 surface.
[Tracking Requested - why for this release]:
this signature has risen to topcrash territory during the last few releases. in firefox 46.0 it is currently making up ~1% of all crashes. maybe we can look into some sort of solution for firefox 47?
Here's a recent one in 49: https://crash-stats.mozilla.com/report/index/4e5c238b-66f1-4a7d-961a-1e3de2160428 lots of out of memory errors (0x8007000E), but they seem to be quite some time before we crash.
This is a recent one in 47: https://crash-stats.mozilla.com/report/index/35047f74-7cfc-4ad7-b9ad-7b64a2160501 which shows what was probably a device reset (we have D3D11+, then D3D11-)

Jeff, is this actionable in any way?
Flags: needinfo?(jmuizelaar)
See Also: → 1260770
This is unlikely to be fixed in 46. Let's track for 47 and 48. This seems to become common on beta/release. If there's anything we can do here we should consider uplift
Looking at the crash from comment 10.

Perhaps irrelevant, but we do get the "Invalid draw target specified: 7" (7 is D2D 1.1) messages, and if you search for crashes that have those messages:

https://crash-stats.mozilla.com/search/?product=Firefox&graphics_critical_error=~Invalid+draw+target+type+specified%3A+7&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

the top one is the one from this bug; another one was in bug 1262681, which seems to be OK post landing a patch that was failing for invalid buffers as if they were null buffers.

Except that DrawTargetCairo::FillGlyphs already does check IsValid(), which should check if mContext is valid as well.

Lee, take a quick look?
Flags: needinfo?(jmuizelaar) → needinfo?(lsalzman)
It would appear that most of these recent reports are due to E_OUTOFMEMORY (0x8007000e) popping up from CreateBitmapRenderTarget.

Rather than assume this succeeds, we need to check for the failure and pass it along.
Flags: needinfo?(lsalzman)
Attachment #8750943 - Flags: review?(jmuizelaar)
Comment on attachment 8750943 [details] [diff] [review]
check for failure in cairo _dwrite_draw_glyphs_to_gdi_surface_gdi

Review of attachment 8750943 [details] [diff] [review]:
-----------------------------------------------------------------

I guess this is better than not.
Attachment #8750943 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/f22fc6d53a97
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Hi Jeff, should we uplift this to Aurora48 and Beta47?
Flags: needinfo?(jmuizelaar)
This only landed today.  Further, if this is an out of memory situation, it may just pop up another crash somewhere else, so I'd wait a week or so.
This page makes Firefox crash: http://www.lavoz.com.ar/sucesos/culpas-repartidas-en-el-desastre-del-avion-sol?cx_level=flujo_2

Scrolling down makes Firefox eat up a lot of RAM and then crash. Tested with a brand new profile.

Got an OOM crash with my main profile: https://crash-stats.mozilla.com/report/index/0a47f050-9c1a-4eed-ab37-1750e2160515

Got this crash with a new profile: https://crash-stats.mozilla.com/report/index/65f2e217-6e4e-41b1-aa69-d69e12160515
lopardo, this is with a Nvidia GTX 970 card?  Could you give us the graphics section of about:support?  Also, could you try the latest Firefox Nightly, and see if you get the same crash?
Comment on attachment 8750943 [details] [diff] [review]
check for failure in cairo _dwrite_draw_glyphs_to_gdi_surface_gdi

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: And old crash, but still about 100 per day in beta alone.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low risk, we just return the error state earlier.
[String/UUID change made/needed]:

We need aurora to be able to confirm the fix worked, because we don't have a lot of crashes in the nightly in the first place.
Flags: needinfo?(jmuizelaar)
Attachment #8750943 - Flags: approval-mozilla-aurora?
Assignee: jmuizelaar → lsalzman
Comment on attachment 8750943 [details] [diff] [review]
check for failure in cairo _dwrite_draw_glyphs_to_gdi_surface_gdi

Crash fix, let's uplift to Aurora to decide whether it makes sense to uplift to Beta47 or not. Aurora48+
Attachment #8750943 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi Milan, Lee: This has only been in Aurora for a day but so far for the 2nd signature I don't see any Aurora reports on build 05-24. Do you think we should uplift to Beta now? or wait on another day worth of data?
Flags: needinfo?(milan)
Flags: needinfo?(lsalzman)
Comment on attachment 8750943 [details] [diff] [review]
check for failure in cairo _dwrite_draw_glyphs_to_gdi_surface_gdi

We are seeing early indications that this patch cleaned things up in Aurora.  I would somewhat speculatively suggest we uplift this to Beta at this point.  We have handled out of memory type of things during printing in some earlier work, so we should be ready for this situation from other workflows.
Flags: needinfo?(milan)
Attachment #8750943 - Flags: approval-mozilla-beta?
Comment on attachment 8750943 [details] [diff] [review]
check for failure in cairo _dwrite_draw_glyphs_to_gdi_surface_gdi

The early data from Aurora48 doesn't show this crash showing up after 05-24 build, and it's ~600 instances on 47.0b6 + b7, makes sense to uplift to Beta47.
Attachment #8750943 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ritu Kothari (:ritu) from comment #24)
> Hi Milan, Lee: This has only been in Aurora for a day but so far for the 2nd
> signature I don't see any Aurora reports on build 05-24. Do you think we
> should uplift to Beta now? or wait on another day worth of data?

The patch should in principle get rid of the crash signature. The unknown is if, by punting the OOM out of Cairo, we will see some new crash or bug in its place. We just have no idea what it would look like right now, other than I guess our standard OOM black-content mess or similar. Ideally, any of our code that uses Cairo is expected to be resilient to any error statuses Cairo reports like this, so the hope is it will fail gracefully now.

But this is all still better than crashing because of a null pointer dereference like we were before.

So, if we feel this crash is annoying enough that it warrants uplifting to beta, I think I would be in favor of uplifting the patch, as confusing as my answer was. :)
Flags: needinfo?(lsalzman)
I don't know if we release note things like this, but if we did:

[Suggested wording]: A high volume crash during printing on lower memory systems has been resolved.
relnote-firefox: --- → ?
(In reply to Milan Sreckovic [:milan] from comment #29)
> I don't know if we release note things like this, but if we did:
> 
> [Suggested wording]: A high volume crash during printing on lower memory
> systems has been resolved.

We don't typically, thus I have added the minus flag here.
Crash volume for signature '_dwrite_draw_glyphs_to_gdi_surface_gdi':
 - nightly (version 50): 0 crash from 2016-06-06.
 - aurora  (version 49): 0 crash from 2016-06-07.
 - beta    (version 48): 0 crash from 2016-06-06.
 - release (version 47): 6 crashes from 2016-05-31.
 - esr     (version 45): 2915 crashes from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly          0          0          0          0          0          0          0
 - aurora           0          0          0          0          0          0          0
 - beta             0          0          0          0          0          0          0
 - release          3          1          0          0          0          1          0
 - esr            332        266        315        331        331        349        212

Affected platform: Windows
[Tracking Requested - why for this release]:

Signature report for _dwrite_draw_glyphs_to_gdi_surface_gdi

Showing results from 6 months ago

Operating System
Windows 7 	22781 	96.1%
Windows 10 	405 	1.7%
Windows 8.1 	341 	1.4%
Windows 8 	158 	0.7%
Windows Vista 	13 	0.1%

Product
Firefox 	45.9.0esr 	724 	63.8% 	782
Firefox 	45.8.0esr 	108 	9.5% 	76
Firefox 	45.3.0esr 	37 	3.3% 	33
Firefox 	45.4.0esr 	35 	3.1% 	27
Firefox 	45.2.0esr 	30 	2.6% 	24
Firefox 	45.7.0esr 	28 	2.5% 	27
Firefox 	45.6.0esr 	21 	1.9% 	14
Firefox 	45.1.1esr 	20 	1.8% 	22
Firefox 	45.5.1esr 	20 	1.8% 	19
Firefox 	46.0.1 		13 	1.1% 	14

Architecture
x86 	23662 	99.8%
amd64 	36 	0.2%
OS: Windows 7 → Windows
esr 45 is dead, please use esr 52
You need to log in before you can comment on or make changes to this bug.