1478084 - Crash in _cairo_user_data_array_set_data.cold.16

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is
report bp-394c6ef7-a809-4f76-88dd-2ce150180724.
=============================================================

Top 10 frames of crashing thread:

0 libxul.so _cairo_user_data_array_set_data.cold.16 
1 libxul.so mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData gfx/2d/ScaledFontFontconfig.cpp:466
2 libxul.so mozilla::gfx::UnscaledFontFontconfig::CreateScaledFont gfx/2d/ScaledFontFontconfig.cpp:374
3 libxul.so mozilla::gfx::RecordedScaledFontCreationByIndex::PlayEvent const gfx/2d/RecordedEventImpl.h:3292
4 libxul.so mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::InlineTranslator::TranslateRecording> > gfx/2d/InlineTranslator.cpp:84
5 libxul.so mozilla::gfx::InlineTranslator::TranslateRecording gfx/2d/InlineTranslator.cpp:89
6 libxul.so mozilla::wr::Moz2DRenderCallback gfx/webrender_bindings/Moz2DImageRenderer.cpp:323
7 libxul.so wr_moz2d_render_cb gfx/webrender_bindings/Moz2DImageRenderer.cpp:369
8 libxul.so rayon::iter::plumbing::bridge_producer_consumer::helper gfx/webrender_bindings/src/moz2d_renderer.rs:375
9 libxul.so <rayon_core::job::StackJob<L, F, R> as rayon_core::job::Job>::execute third_party/rust/rayon/src/iter/plumbing/mod.rs:413

=============================================================

There is 1 crash in nightly 63 with buildid 20180723100101. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1455422.

[1] https://hg.mozilla.org/mozilla-central/rev?node=80efe717b563

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Thoughts Lee?

Comment 2

[Lee Salzman [:lsalzman]]

The Cairo font user data array is unfortunately not thread safe. In principle we had previously locked most places that could simultaneously muck with the same font's user data, but it's possible that as a result of the aforementioned bug 1455422 maybe we are now hitting a path that is not properly locking things. I'll try to dig a bit.

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: make cairo_font_face_set_user_data thread-safe

This ensures we're holding the unscaled font's mutex while we mess with any font face user data. To do this it just exposes some backend hooks for locking and unlocking.

In some cases there are some cairo_ft_font_faces that don't remember their unscaled font anymore, not allowing the locking hooks to work, so we need to instead just lock the mutex around the call to cairo_font_face_destroy in those places.

Comment 4

[Lee Salzman [:lsalzman]]

Attachment: fix recursive locking in SkFontHost_cairo

There is one place where this new internal locking inside cairo_font_face_set_user_data can cause us to recursively lock if we're already inside the scope of cairo_ft_scaled_font_lock_face.

However, in the one instance, we don't actually need to pass a valid SkFontStyle to Skia, since SkFontHost_cairo doesn't use any of that. So we can just get rid of the cairo_ft_scaled_font_lock_face call here and all is well.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac9f181dfe5a
make cairo_font_face_set_user_data thread-safe. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/d837831ad47c
fix recursive locking in SkFontHost_cairo. r=jrmuizel

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ac9f181dfe5a
https://hg.mozilla.org/mozilla-central/rev/d837831ad47c