1552687 - Crash in [@ ComPtr<T>::~ComPtr<T>]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Julian Seward [:jseward]]

This bug is for crash report bp-0d302527-018f-4e36-85e3-5e3ae0190518.

This crash appeared in two different installations in the Windows nightly
of 20190516093926.

Top 10 frames of crashing thread:

0 dwrite.dll ComPtr<IDWriteFontFileStream>::~ComPtr<IDWriteFontFileStream> 
1 dwrite.dll FontFileReference::~FontFileReference 
2 dwrite.dll FontFace::~FontFace 
3 dwrite.dll DWriteFontFace::Release 
4 xul.dll static void _cairo_dwrite_font_face_destroy gfx/cairo/cairo/src/cairo-dwrite-font.cpp:324
5 xul.dll moz_cairo_font_face_destroy gfx/cairo/cairo/src/cairo-font-face.c:150
6 xul.dll void _cairo_scaled_font_fini_internal gfx/cairo/cairo/src/cairo-scaled-font.c:834
7 xul.dll moz_cairo_scaled_font_destroy gfx/cairo/cairo/src/cairo-scaled-font.c:1277
8 xul.dll void gfxDWriteFont::~gfxDWriteFont gfx/thebes/gfxDWriteFonts.cpp:103
9 xul.dll void gfxDWriteFont::~gfxDWriteFont gfx/thebes/gfxDWriteFonts.cpp:98

Comment 1

[Jonathan Kew [:jfkthame]]

This doesn't appear to be a new issue. E.g. https://crash-stats.mozilla.org/report/index/bb5f5edd-dec4-4765-85f1-134840190515 is a very similar-looking crash from 60esr:

0 dwrite.dll 	ComPtr<IDWriteFontFileStream>::~ComPtr<IDWriteFontFileStream>()
1 dwrite.dll 	FontFileReference::~FontFileReference()
2 dwrite.dll 	FontFace::~FontFace()
3 dwrite.dll 	DWriteFontFace::Release()
4 xul.dll 	_cairo_dwrite_font_face_destroy 	gfx/cairo/cairo/src/cairo-dwrite-font.cpp:324
5 xul.dll 	moz_cairo_font_face_destroy 	gfx/cairo/cairo/src/cairo-font-face.c:150
6 xul.dll 	_cairo_scaled_font_fini_internal 	gfx/cairo/cairo/src/cairo-scaled-font.c:834
7 xul.dll 	moz_cairo_scaled_font_destroy 	gfx/cairo/cairo/src/cairo-scaled-font.c:1277
8 xul.dll 	gfxDWriteFont::~gfxDWriteFont() 	gfx/thebes/gfxDWriteFonts.cpp:93
9 xul.dll 	gfxDWriteFont::`scalar deleting destructor'(unsigned int)

I don't have any particular insight into how to investigate/debug this, unfortunately. If we had a reproducible testcase that'd be different, but I suspect this is a fairly rare intermittent issue.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: Bug 1552687 - guard access to gfxDWriteFontFileStream with mutex. r?jrmuizel

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57626db46158
guard access to gfxDWriteFontFileStream with mutex. r=jfkthame

Comment 4

[Lee Salzman [:lsalzman]]

It looks like this pattern of use-after-free ~ComPtr crashes started in 58, and that is also when we enabled OMTP for Windows. That would make sense if this is a race caused by destruction of the font via the expiration timer and the paint thread. Removing the global lock in Skia's dwrite code may have just aggravated the problem, but I don't believe is the hypothetical cause.

Hopefully mutexing this more carefully will resolve the issue well enough that we can remove the Skia global locking again.

Comment 5

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/57626db46158

Comment 6

[Lee Salzman [:lsalzman]]

Comment on attachment 9066846 [details]
Bug 1552687 - guard access to gfxDWriteFontFileStream with mutex. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Race condition leading to crash on Windows if OMTP or WebRender is enabled.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just fixes race condition with some extra locking. Eliminated crashes in nightly.
• String changes made/needed:

Comment 8

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9066846 [details]
Bug 1552687 - guard access to gfxDWriteFontFileStream with mutex. r?jrmuizel

locking fix, let's see how this goes in 68.0b8

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/44474b3668a5