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

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
critical
RESOLVED FIXED
2 months ago
Last month

People

(Reporter: jseward, Assigned: lsalzman)

Tracking

(Regression, {crash, regression})

unspecified
mozilla69
Unspecified
Windows 10
Points:
---

Firefox Tracking Flags

(firefox-esr60 wontfix, firefox67 wontfix, firefox68 fixed, firefox69 fixed)

Details

(crash signature)

Attachments

(1 attachment)

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

Flags: needinfo?(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.

Flags: needinfo?(jfkthame)
Priority: -- → P3
Regressed by: 1533546
Crash Signature: [@ ComPtr<T>::~ComPtr<T>] → [@ ComPtr<T>::~ComPtr<T>] [@ ComPtr<T>::ComPtr<T>]
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57626db46158
guard access to gfxDWriteFontFileStream with mutex. r=jfkthame

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.

Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Regressed by: 1403935
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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:
Attachment #9066846 - Flags: approval-mozilla-beta?
Duplicate of this bug: 1544217

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

Attachment #9066846 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.