606419 - crash on Windows 7 [@ gfxDWriteShaper::InitTextRun(gfxContext*, gfxTextRun*, unsigned short const*, unsigned int, unsigned int, int) ]

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Scoobidiver (away)]

It is a residual crash signature that exists in trunk build.
It happens only on Windows 7.
It is #133 top crasher in 4.0b8pre for the last week.

Signature	gfxDWriteShaper::InitTextRun(gfxContext*, gfxTextRun*, unsigned short const*, unsigned int, unsigned int, int)
UUID	9760c168-f129-47d2-a013-823602101021
Time 	2010-10-21 13:28:39.82864
Uptime	518
Last Crash	549 seconds (9.2 minutes) before submission
Install Age	10912 seconds (3.0 hours) since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101021042123
Branch	2.0
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	GenuineIntel family 6 model 23 stepping 6
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 	AdapterVendorID: 1002, AdapterDeviceID: 9442

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	gfxDWriteShaper::InitTextRun 	gfx/thebes/gfxDWriteShaper.cpp:72
1 	xul.dll 	gfxFont::InitTextRun 	gfx/thebes/gfxFont.cpp:1373
2 	xul.dll 	gfxFontGroup::InitTextRun 	gfx/thebes/gfxFont.cpp:2276
3 	xul.dll 	gfxFontGroup::InitTextRun 	gfx/thebes/gfxFont.cpp:2244
4 	xul.dll 	gfxFontGroup::MakeTextRun 	gfx/thebes/gfxFont.cpp:2212
5 	xul.dll 	TextRunWordCache::MakeTextRun 	gfx/thebes/gfxTextRunWordCache.cpp:693
6 	xul.dll 	MakeTextRun 	layout/generic/nsTextFrameThebes.cpp:504
7 	xul.dll 	BuildTextRunsScanner::BuildTextRunForFrames 	layout/generic/nsTextFrameThebes.cpp:1864
8 	xul.dll 	BuildTextRunsScanner::FlushFrames 	layout/generic/nsTextFrameThebes.cpp:1296
9 	xul.dll 	BuildTextRuns 	layout/generic/nsTextFrameThebes.cpp:1230
10 	xul.dll 	nsTextFrame::EnsureTextRun 	layout/generic/nsTextFrameThebes.cpp:2120
11 	xul.dll 	nsTextFrame::AddInlineMinWidthForFlow 	layout/generic/nsTextFrameThebes.cpp:5888
12 	xul.dll 	nsTextFrame::AddInlineMinWidth 	layout/generic/nsTextFrameThebes.cpp:6000
13 	xul.dll 	nsBlockFrame::GetMinWidth 	layout/generic/nsBlockFrame.cpp:743
14 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
15 	xul.dll 	nsTableCellFrame::GetMinWidth 	layout/tables/nsTableCellFrame.cpp:729
16 	xul.dll 	GetWidthInfo 	layout/tables/BasicTableLayoutStrategy.cpp:113
17 	xul.dll 	BasicTableLayoutStrategy::ComputeColumnIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:308
18 	xul.dll 	BasicTableLayoutStrategy::ComputeIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:418
19 	xul.dll 	BasicTableLayoutStrategy::GetMinWidth 	layout/tables/BasicTableLayoutStrategy.cpp:74
20 	xul.dll 	nsTableFrame::GetMinWidth 	layout/tables/nsTableFrame.cpp:1491
21 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
22 	xul.dll 	nsTableOuterFrame::GetMinWidth 	layout/tables/nsTableOuterFrame.cpp:501
23 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
24 	xul.dll 	nsBlockFrame::GetMinWidth 	layout/generic/nsBlockFrame.cpp:724
25 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
26 	xul.dll 	nsBlockFrame::GetMinWidth 	layout/generic/nsBlockFrame.cpp:724
27 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
28 	xul.dll 	nsBlockFrame::GetMinWidth 	layout/generic/nsBlockFrame.cpp:724
29 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
30 	xul.dll 	nsTableCellFrame::GetMinWidth 	layout/tables/nsTableCellFrame.cpp:729
31 	xul.dll 	GetWidthInfo 	layout/tables/BasicTableLayoutStrategy.cpp:113
32 	xul.dll 	BasicTableLayoutStrategy::ComputeColumnIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:308
33 	xul.dll 	BasicTableLayoutStrategy::ComputeIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:418
34 	xul.dll 	BasicTableLayoutStrategy::GetMinWidth 	layout/tables/BasicTableLayoutStrategy.cpp:74
35 	xul.dll 	nsTableFrame::GetMinWidth 	layout/tables/nsTableFrame.cpp:1491
36 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
...
52 	xul.dll 	nsLayoutUtils::IntrinsicForContainer 	layout/base/nsLayoutUtils.cpp:2012
53 	xul.dll 	nsTableCellFrame::GetMinWidth 	layout/tables/nsTableCellFrame.cpp:729
54 	xul.dll 	GetWidthInfo 	layout/tables/BasicTableLayoutStrategy.cpp:113
55 	xul.dll 	BasicTableLayoutStrategy::ComputeColumnIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:308
56 	xul.dll 	BasicTableLayoutStrategy::ComputeIntrinsicWidths 	layout/tables/BasicTableLayoutStrategy.cpp:418
57 	xul.dll 	BasicTableLayoutStrategy::GetMinWidth 	layout/tables/BasicTableLayoutStrategy.cpp:74
58 	xul.dll 	nsTableFrame::GetMinWidth 	layout/tables/nsTableFrame.cpp:1491
59 	xul.dll 	nsTableFrame::TableShrinkWidthToFit 	layout/tables/nsTableFrame.cpp:1546
60 	xul.dll 	nsTableFrame::ComputeAutoSize 	layout/tables/nsTableFrame.cpp:1578
61 	xul.dll 	nsFrame::ComputeSize 	layout/generic/nsFrame.cpp:3355
62 	xul.dll 	nsTableFrame::ComputeSize 	layout/tables/nsTableFrame.cpp:1531
63 	xul.dll 	ChildShrinkWrapWidth 	layout/tables/nsTableOuterFrame.cpp:584
64 	xul.dll 	nsTableOuterFrame::ComputeAutoSize 	layout/tables/nsTableOuterFrame.cpp:611
65 	xul.dll 	nsFrame::ComputeSize 	layout/generic/nsFrame.cpp:3355
66 	xul.dll 	nsBlockFrame::WidthToClearPastFloats 	layout/generic/nsBlockFrame.cpp:7008
67 	xul.dll 	nsBlockReflowState::ClearFloats 	layout/generic/nsBlockReflowState.cpp:1004
68 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:3052
69 	xul.dll 	nsBlockFrame::ReflowLine 	layout/generic/nsBlockFrame.cpp:2470
70 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:1963
71 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:1040
72 	xul.dll 	nsBlockReflowContext::ReflowBlock 	layout/generic/nsBlockReflowContext.cpp:297
73 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:3148
74 	xul.dll 	nsBlockFrame::ReflowLine 	layout/generic/nsBlockFrame.cpp:2470
75 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:1963
76 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:1040
...
92 	xul.dll 	nsBlockReflowContext::ReflowBlock 	layout/generic/nsBlockReflowContext.cpp:297
93 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:3148
94 	xul.dll 	nsBlockFrame::ReflowLine 	layout/generic/nsBlockFrame.cpp:2470
95 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:1963
96 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:1040
97 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:739
98 	xul.dll 	nsCanvasFrame::Reflow 	layout/generic/nsCanvasFrame.cpp:494
99 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:739
100 	xul.dll 	nsHTMLScrollFrame::ReflowScrolledFrame 	layout/generic/nsGfxScrollFrame.cpp:522
101 	xul.dll 	nsHTMLScrollFrame::ReflowContents 	layout/generic/nsGfxScrollFrame.cpp:614
102 	xul.dll 	nsHTMLScrollFrame::Reflow 	layout/generic/nsGfxScrollFrame.cpp:822
103 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:739
104 	xul.dll 	ViewportFrame::Reflow 	layout/generic/nsViewportFrame.cpp:293
105 	xul.dll 	PresShell::DoReflow 	layout/base/nsPresShell.cpp:7751
106 	xul.dll 	PresShell::ProcessReflowCommands 	layout/base/nsPresShell.cpp:7890
107 	xul.dll 	PresShell::FlushPendingNotifications 	layout/base/nsPresShell.cpp:4869
108 	xul.dll 	nsRefreshDriver::Notify 	layout/base/nsRefreshDriver.cpp:310
109 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:428
110 	xul.dll 	nsTimerEvent::Run 	xpcom/threads/nsTimerImpl.cpp:517
111 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:547
112 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:134
113 	xul.dll 	xul.dll@0xb011b3 	
114 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:202
115 	xul.dll 	_SEH_epilog4 	
116 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:176
117 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:180
118 	xul.dll 	xul.dll@0xb011b3 	
119 	xul.dll 	nsAppShell::Run 	widget/src/windows/nsAppShell.cpp:243
120 	DWrite.dll 	sfac_SearchForBitmap 	
121 		@0x75edffff 	
122 	dui70.dll 	_chkstk 	
123 	DWrite.dll 	sfac_SearchForBitmap 	
124 	explorerframe.dll 	explorerframe.dll@0x6736d 	
125 	DWrite.dll 	sfac_SearchForBitmap 	
126 		@0x7538ffff 	

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=gfxDWriteShaper%3A%3AInitTextRun%28gfxContext*%2C%20gfxTextRun*%2C%20unsigned%20short%20const*%2C%20unsigned%20int%2C%20unsigned%20int%2C%20int%29

Comment 1

[Jonathan Kew [:jfkthame]]

It looks to me like this could happen if the browser is initially running in d2d+dwrite mode, and then gfxWindowsPlatform::UpdateRenderMode() gets called (from nsWindow::OnPaint() at http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindowGfx.cpp#724 or http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindowGfx.cpp#733), and d2d has been disabled via prefs in the meantime (and gfx.font_rendering.directwrite.enabled is not explicitly set to TRUE).

In this scenario, UpdateRenderMode() will end up setting mUseDirectWrite to FALSE, which means that gfxWindowsPlatform::GetDWriteFactory() will always return NULL, but we actually have a DWrite-based font list. Thus, we'll keep using gfxDWriteShaper, but when it tries to access the factory (to request an IDWriteTextAnalyzer), the factory pointer returned by the platform will be NULL.

To fix this, I think we need to either discard and re-create the platform's font-list when the render-mode changes (and ensure all existing gfxFont instances are flushed), so that we'll switch fully to GDI if the new render-mode and prefs don't call for DWrite; or else GetDWriteFactory() needs to keep returning the real IDWriteFactory pointer even when the preferences have changed, so that we keep using DWrite until the browser is actually restarted.

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to comment #1)
> It looks to me like this could happen if the browser is initially running in
> d2d+dwrite mode, and then gfxWindowsPlatform::UpdateRenderMode() gets called
> (from nsWindow::OnPaint() at
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindowGfx.cpp#724
> or
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindowGfx.cpp#733),
> and d2d has been disabled via prefs in the meantime (and
> gfx.font_rendering.directwrite.enabled is not explicitly set to TRUE).
> 
> In this scenario, UpdateRenderMode() will end up setting mUseDirectWrite to
> FALSE, which means that gfxWindowsPlatform::GetDWriteFactory() will always
> return NULL, but we actually have a DWrite-based font list. Thus, we'll keep
> using gfxDWriteShaper, but when it tries to access the factory (to request an
> IDWriteTextAnalyzer), the factory pointer returned by the platform will be
> NULL.
> 
> To fix this, I think we need to either discard and re-create the platform's
> font-list when the render-mode changes (and ensure all existing gfxFont
> instances are flushed), so that we'll switch fully to GDI if the new
> render-mode and prefs don't call for DWrite; or else GetDWriteFactory() needs
> to keep returning the real IDWriteFactory pointer even when the preferences
> have changed, so that we keep using DWrite until the browser is actually
> restarted.

How hard would it be to flush the gfxFont instances? I believe this would be the preferred approach but I'm not sure how hard it would be.

Comment 3

[Joe Drew (not getting mail)]

Jonathan, just passing this to you so Bas can get an answer to his question in comment 2. If you're not the right person to fix this, pass it to someone who is :)

Comment 4

[Kevin Brosnan]

I crashed on this, I had toggled the Use hardware acceleration when available on and off a couple times without restarting. While unchecked I moused over a number of themes at getpersonas.com.

Adapter Description NVIDIA GeForce GT 240M
Vendor ID 10de
Device ID 0a34
Adapter RAM 1024
Adapter Drivers nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Driver Version 8.17.12.6063
Driver Date 9-10-2010
Direct2D Enabled false
DirectWrite Enabled false
GPU Accelerated Windows 1/2 Direct3D 9

Trying to reproduce.

Comment 5

[Scoobidiver (away)]

It is #46 top crasher in 4.0b8pre for the last week.

There are a lot of comments. See:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=gfxDWriteShaper%3A%3AInitTextRun%28gfxContext*%2C%20gfxTextRun*%2C%20unsigned%20short%20const*%2C%20unsigned%20int%2C%20unsigned%20int%2C%20int%29

Comment 6

[Jonathan Kew [:jfkthame]]

A lot of the comments mention changing HW accel prefs, or installing/updating drivers, which seems to support the interpretation in comment #1. I'm looking into what we can do...

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to comment #2)

> > To fix this, I think we need to either discard and re-create the platform's
> > font-list when the render-mode changes (and ensure all existing gfxFont
> > instances are flushed), so that we'll switch fully to GDI if the new
> > render-mode and prefs don't call for DWrite; or else GetDWriteFactory() needs
> > to keep returning the real IDWriteFactory pointer even when the preferences
> > have changed, so that we keep using DWrite until the browser is actually
> > restarted.
> 
> How hard would it be to flush the gfxFont instances? I believe this would be
> the preferred approach but I'm not sure how hard it would be.

I've been experimenting a bit with this, and it looks to me like it'd be hard to do this. We can flush the textrun cache and forcibly expire fonts from the gfxFontCache, so that new instances get created from the fontEntries (of the new fontList) when needed, but I don't see how we'd deal with the fact that existing gfxFontGroup objects also have references to the gfxFont instances that resulted from resolving their family names and styles, and those would be the "wrong" kind of font.

So I think the safe option, at least for the short term, is to say that once we've initialized the font system (using either dwrite or gdi) we don't change that on the fly. Which means that if you turn off D2D, we'll continue to use DWrite fonts until restart, even if they're not explicitly turned on in prefs.

Note that the question of whether these rendering-mode prefs are "live" is already a bit confused. If we're in D2D mode, we call UpdateRenderMode regularly, and therefore if the user sets the direct2d.disabled pref, we'll notice this and switch to GDI immediately. (And then probably crash when trying to use a DWrite font without a DWrite factory, but that's what this bug is about.)

However, in GDI mode we don't generally call UpdateRenderMode at all, and so toggling back to D2D doesn't take effect until the browser is restarted. So this preference is "live" in one direction but requires a restart to go the other way.

Comment 8

[Jonathan Kew [:jfkthame]]

BTW (for testing purposes), if you set gfx.font_rendering.harfbuzz.level=0, so that all fonts will go through the dwrite shaper rather than the harfbuzz path, then toggling direct2d.disabled to TRUE in a browser that's running in D2D mode will reliably trigger this crash.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: patch, always return the dwrite factory if one has been created

I think this is sufficient to stop the crashes we're seeing; they occur when d2d is turned off in prefs (or disabled because of underlying system/driver changes), which leads to mUseDirectWrite being set to FALSE, but we already have DWrite font objects which continue to need the factory.

Comment 10

[Bas Schouten (:bas.schouten)]

(In reply to comment #9)
> Created attachment 493271 [details] [diff] [review]
> patch, always return the dwrite factory if one has been created
> 
> I think this is sufficient to stop the crashes we're seeing; they occur when
> d2d is turned off in prefs (or disabled because of underlying system/driver
> changes), which leads to mUseDirectWrite being set to FALSE, but we already
> have DWrite font objects which continue to need the factory.

Right if we do this though, all future rendering will -also- be done using DirectWrite, shouldn't we be migrating somehow if people don't -want- directwrite?

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to comment #10)
> Right if we do this though, all future rendering will -also- be done using
> DirectWrite, shouldn't we be migrating somehow if people don't -want-
> directwrite?

I don't think we can switch cleanly without a browser restart. I suppose maybe we could create a GDI font list and start instantiating new fonts as GDI, but keep the DW stuff around so that the existing DW fonts continue to work. But that would give people a really strange, unpredictable mixture of DW and GDI font rendering, not a definite switch. That seems really confusing and unhelpful -- better to simply require a restart for the change to take effect.

Comment 12

[Jonathan Kew [:jfkthame]]

(In reply to comment #11)
> I don't think we can switch cleanly without a browser restart. .....

Thinking a bit more about what it would take to do a "live" switch at runtime - we'd need to flush the textrun cache and the font cache (which is fine, we have methods to do that), but we'd also need to add a check in gfxFontGroup::InitTextRun to detect that the font subsystem has changed, and then make the group re-resolve its font list to get new gfxFont instances. (Not sure offhand if there may be other places that would be similarly affected.) IMO, that's overhead and complexity and risk that we shouldn't be adding for the sake of making an advanced preference be "live-apply".

Comment 13

[Scoobidiver (away)]

> better to simply require a restart for the change to take effect.
See bug 593027 to disable/enable HW acceleration by UI without a restart.

Comment 14

[Bas Schouten (:bas.schouten)]

Comment on attachment 493271 [details] [diff] [review]
patch, always return the dwrite factory if one has been created

We should make sure we track the ability to switch from DWrite to GDI fonts when a problem occurs with D2D as per IRC discussion.

Comment 15

[Jonathan Kew [:jfkthame]]

(In reply to comment #14)
> We should make sure we track the ability to switch from DWrite to GDI fonts
> when a problem occurs with D2D as per IRC discussion.

Filed bug 615905 on this.

Comment 16

[Jonathan Kew [:jfkthame]]

Pushed http://hg.mozilla.org/mozilla-central/rev/f257e2a6cdcf, which I believe should prevent these crashes. See bug 615905 for the wider issue of switching font back-ends when we disable D2D on the fly.