1100949 - crash in gfxFontUtils::ReadNames(char const*, unsigned int, unsigned int, int, int, nsTArray<nsString>&)

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-c3d5c69b-4712-4d67-abcd-849492141116.
=============================================================

Top frames:
0 	xul.dll 	gfxFontUtils::ReadNames(char const*, unsigned int, unsigned int, int, int, nsTArray<nsString>&) 	gfx/thebes/gfxFontUtils.cpp
1 	dwrite.dll 	_SEH_epilog4_GS 	
2 	xul.dll 	FontInfoData::Load() 	gfx/thebes/gfxFontInfoLoader.cpp
3 	xul.dll 	AsyncFontInfoLoader::Run() 	gfx/thebes/gfxFontInfoLoader.cpp
4 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp


Top URLs:
54 	https://www.facebook.com/
9 	https://www.facebook.com/jayar.a.canas
There's also a number of YouTube URLs on there, and a wide range of single-hit URLs.

All those crashes report a reason of EXCEPTION_IN_PAGE_ERROR, strangely, the vast majority of crashes are in the third minute of the process being up, between 120 and 180 seconds, all addresses seem to be in the range between 0x50000000 and 0x70000000.
The crashes are all on Win7 or Vista, but on all kinds of Firefox versions, and there's no discernible pattern of graphics adapters.

Comment 1

[Robert Kaiser]

See https://crash-stats.mozilla.com/report/list?signature=gfxFontUtils%3A%3AReadNames%28char%20const%2A%2C%20unsigned%20int%2C%20unsigned%20int%2C%20int%2C%20int%2C%20nsTArray%3CnsString%3E%26%29 for more stats and crash reports.

Comment 2

[John Daggett (:jtd)]

This is in the async font data loader thread. So the page loading probably isn't relevant. The crash is reading font name data within the DirectWrite backend (that's why this is only Vista/Win7).

gfxFontUtils::ReadNames(char const*, unsigned int, unsigned int, int, int, nsTArray<nsString>&) 	gfx/thebes/gfxFontUtils.cpp:1384
gfxFontUtils::ReadCanonicalName(char const*, unsigned int, unsigned int, nsString&) 	gfx/thebes/gfxFontUtils.cpp:1145
DirectWriteFontInfo::LoadFontFamilyData(nsAString_internal const&) 	gfx/thebes/gfxDWriteFontList.cpp:1697
FontInfoData::Load() 	gfx/thebes/gfxFontInfoLoader.cpp:23
AsyncFontInfoLoader::Run()

The EXCEPTION_IN_PAGE_ERROR is interesting, need to figure out what that means precisely. DirectWrite has already handed us back the name table and we check that the font table lookup method didn't report an error or return a table with no data.

Comment 3

[(Away)]

These are coming from a small number of users (judging by install time). There is one user who has sent 445 reports!

Interpreting the exception details according to [1]:
* This was a read operation
* The failing address was inside a MEM_MAPPED region
* The reason is: 0xc0000006 - The instruction at 0x%p referenced memory at 0x%p. The required data was not placed into memory because of an I/O error status of 0x%x.

I am going to guess that |aNamedData| is a memory-mapped font file? I bet it's in a bad sector on the affected machines.

[1] http://msdn.microsoft.com/en-us/library/aa363082%28VS.85%29.aspx

Comment 4

[John Daggett (:jtd)]

This still happens, judging from 41.0b beta builds:

https://crash-stats.mozilla.com/signature/?product=Firefox&version=41.0b&signature=gfxFontUtils%3A%3AReadNames%28char+const*%2C+unsigned+int%2C+unsigned+int%2C+int%2C+int%2C+nsTArray%3CT%3E%26%29&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&page=1#reports

I think the solution for this is probably to use structured exception handling do deal with these file mapping exceptions. Searching through crash reports with reason = EXCEPTION_IN_PAGE_ERROR, this seems to occur both within our code within shaping/drawing code *and* within DirectWrite code.

The font loader thread is the reason this specific bug occurs. I think the solution here is just to catch the exception using an MS-specific __try block and just ignore the font data. Guessing this is basically a DirectWrite bug that we can't really avoid but at least we can avoid tickling it so often.

Comment 5

[John Daggett (:jtd)]

Attachment: patch, wrap font info reading with exception handler

Untested. Wrap DirectWrite font info loader code with __try {} block.

Comment 6

[John Daggett (:jtd)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a01b13e8758e

Comment 7

[John Daggett (:jtd)]

Attachment: patch, use exception handling around font info loads

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 8655284 [details] [diff] [review]
patch, use exception handling around font info loads

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

::: gfx/thebes/gfxFontInfoLoader.cpp
@@ +28,5 @@
>      for (i = 0; i < n; i++) {
> +        // font file memory mapping sometimes causes exceptions - bug 1100949
> +        MOZ_SEH_TRY {
> +            LoadFontFamilyData(mFontFamiliesToLoad[i]);
> +        } MOZ_SEH_EXCEPT(EXCEPTION_EXECUTE_HANDLER) {}

Please add a gfxCriticalError() here so we have some idea of if this exception might correlate with other crashes we end up seeing when this is fixed.

Comment 9

[John Daggett (:jtd)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca013116e54addc78f093eee7926a1f84c3747ea
Bug 1100949 - wrap font info reads with structured exception handler. r=bas

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ca013116e54a