1412545 - regression: blob-images: libfreetype crashes (broken webrender_bindings)

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Darkspirit]

Nightly 58 x64 20171028100423 @ Debian Testing (KDE, Radeon RX480)
main profile: gpu process, layers force accel, webrender, blob-images, omtp,
+ new since today: layout.css.servo.chrome.enabled;true (bug 1411532)

I don't know what caused this. But you can see webrender in the crash stacks. For example:
> core::ptr::drop_in_place<std::sync::mpsc::Sender<(webrender_api::image::BlobImageRequest, core::result::Result<webrender_api::image::RasterizedBlobImage, webrender_api::image::BlobImageError>)>>

Meldungs-ID 	Sendedatum
bp-523006c1-3bc1-4732-9f2d-681950171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x5efda | libfreetype.so.6.14.0@0x62107 | libfreetype.so.6.14.0@0x15b0e | libfreetype.so.6.14.0@0x63b0f | libfreetype.so.6.14.0@0x2b1a5f | libfreetype.so.6.14.0@0x2b2c7f ]

bp-448ac951-7957-4a70-b7a6-4e6130171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x18738 | mozilla::gfx::Factory::ReleaseFTFace ]

bp-4478996c-6bf5-4d27-be3b-e81480171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x5efda | libfreetype.so.6.14.0@0x62107 | libfreetype.so.6.14.0@0x15b0e | libfreetype.so.6.14.0@0x63b0f | libfreetype.so.6.14.0@0x2b1a5f | libfreetype.so.6.14.0@0x2b2c7f ]

bp-35fb4fed-ff91-408d-9a55-2d7f40171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x5efda | libfreetype.so.6.14.0@0x62107 | libfreetype.so.6.14.0@0x15b0e | libfreetype.so.6.14.0@0x63b0f | libfreetype.so.6.14.0@0x2b1a5f | libfreetype.so.6.14.0@0x2b2c7f ]

bp-56c82642-6431-4939-a692-779c40171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x5efda | libfreetype.so.6.14.0@0x62107 | libfreetype.so.6.14.0@0x15b0e | libfreetype.so.6.14.0@0x63b0f | libfreetype.so.6.14.0@0x2b1a5f | libfreetype.so.6.14.0@0x2b2c7f ]

bp-6c071a93-bed8-4e15-8024-60ede0171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x18738 | mozilla::gfx::Factory::ReleaseFTFace ]

bp-5513d9bc-635b-42d1-9812-215010171028 28.10.17 19:29
[@ mozalloc_abort | abort | _cairo_array_allocate.cold.19 ]

bp-6ab279e4-7f69-4eb4-bee0-2af890171028 28.10.17 19:29
[@ libfreetype.so.6.14.0@0x18738 | mozilla::gfx::Factory::ReleaseFTFace ]

Comment 1

[Darkspirit]

Nightly 58 x64 20171028100423 de_DE @ Debian Testing (KDE, Radeon RX480)
FRESH PROFILE: layers.acceleration.force-enabled, gfx.webrender.enabled, gfx.webrender.blob-images

STR: Click on the library button very often. Maybe open one tab with bugzilla or about:preferences.
Crash signatures differ. All but one crashes contain something from "webrender_bindings".

On each click on the library button I get a new
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
and sometimes I get a crash.

> darkspirit@darkspirit:~$ firefox/firefox -P wr
> ATTENTION: default value of option force_s3tc_enable overridden by environment.
> ATTENTION: default value of option force_s3tc_enable overridden by environment.
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> WebRender - OpenGL version new 4.5 (Core Profile) Mesa 17.2.3
> ExceptionHandler::GenerateDump cloned child 20208
> ExceptionHandler::SendContinueSignalToChild sent continue signal to child
> ExceptionHandler::WaitForContinueSignal waiting for continue signal...
> ERROR:audioipc_server: server poll error: Unterbrechung während des Betriebssystemaufrufs (os error 4)

Meldungs-ID 	Sendedatum
bp-eb103a03-ab6a-4a23-b6a6-9dc3d0171028 28.10.17 20:40
[@ _cairo_user_data_array_set_data.cold.16 ]

bp-0638891e-e15f-4b21-a597-1b1830171028 28.10.17 20:40
[@ libfreetype.so.6.14.0@0x156c0 | libfreetype.so.6.14.0@0x15784 | SkScalerContext_CairoFT::generateMetrics ]

bp-07b7cff4-61dc-4a12-b15b-1fba40171028 28.10.17 20:40
[@ mozalloc_abort | abort | _cairo_array_allocate.cold.19 ]

bp-e7a43055-a30a-47ef-82ca-c64830171028 28.10.17 20:40
[@ mozalloc_abort | abort | _cairo_array_allocate.cold.19 ]

bp-50a841fc-90bd-4d6a-988e-a29cc0171028 28.10.17 20:38
[@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]

Comment 2

[Darkspirit]

layers.acceleration.force-enabled, gfx.webrender.enabled, gfx.webrender.blob-images

open Wikipedia from about:newtab, click on the library button often/fast

mozregression --profile-persistence clone-first --good 2017-10-19 --bad 2017-10-28
> 11:05.66 INFO: No more inbound revisions, bisection finished.
> 11:05.66 INFO: Last good revision: 6aeb83b2225071a5daeacc8ad2b02e92018fd7f6
> 11:05.66 INFO: First bad revision: ad716d87938ffc58d73a4985def6ec66f68e9c2f
> 11:05.66 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6aeb83b2225071a5daeacc8ad2b02e92018fd7f6&tochange=ad716d87938ffc58d73a4985def6ec66f68e9c2f
> 
> 11:07.06 ERROR: Unable to exploit the merge commit. Origin branch is mozilla-inbound, and the commit message for ad716d87 was:
> Merge m-c to m-i
> 
> MozReview-Commit-ID: 2FUpmxEyNnH

I suspect:
> Jeff Muizelaar — Bug 1380014. Keep the unscaled font alive. r=lsalzman

This bug should "block bug 1380014" because it's a regression,
but it also "depends on bug 1380014 comment 17" ("Fix up webrender bindings").

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: AddRef user data before passing to cairo_font_face_set_user_data

Going from this stack trace: https://crash-stats.mozilla.com/report/index/6c071a93-bed8-4e15-8024-60ede0171028

A possible causal chain:

cairo_ft_font_face_create_for_pattern checks a hash table for reuse of the unscaled font, and then within the unscaled font looks through a list of scaled fonts with matching attributes, and thus may finally return that.

In that case, we're trying to set user data on the same Cairo font face which already has this user data set. When Cairo sees this user data with matching key, it calls the release function on the old user data before setting the new key. So if the NFR only has one ref to it by this point, we may end up accidentally freeing it.

To get around this, we must take care to AddRef the NFR before setting user data. Release the ref only if we failed to set the user data (where the release function would otherwise not get called).

I'm not certain this is the cause, but this is none the less a potential bug that matches the trace, so it is good to fix this regardless.

Comment 7

[Darkspirit]

(In reply to Lee Salzman [:lsalzman] from comment #3)
> Going from this stack trace: https://crash-stats.mozilla.com/report/index/6c071a93-bed8-4e15-8024-60ede0171028

This one looks similar (has "free" in the crash stack), but has a different crash signature:
bp-4c9722ab-68e0-429c-8b38-dff5f0171029 [@ libfontconfig.so.1.9.3@0x1efcc ]

Comment 8

[Darkspirit]

Nightly 58 x64 20171030103605 de_DE @ Debian Testing (KDE, Radeon RX480)
main profile
about:buildconfig > https://hg.mozilla.org/mozilla-central/rev/515407ebfa1433c31144374313bbfd8b942af41c

According to https://hg.mozilla.org/mozilla-central/graph/515407ebfa1433c31144374313bbfd8b942af41c I have the patch from bug 1380014 comment 17 ("Fix up webrender bindings").

Meldungs-ID 	Sendedatum
bp-541aa467-fee5-4df2-88e8-3a8110171031 31.10.17 03:08
[@ libfreetype.so.6.15.0@0x13580 | libfreetype.so.6.15.0@0x13644 | _moz_cairo_ft_scaled_font_lock_face ]

bp-759efc7f-0635-4af6-b58e-6b7890171031 31.10.17 02:59
[@ libfreetype.so.6.15.0@0x13580 | libfreetype.so.6.15.0@0x13644 | _moz_cairo_ft_scaled_font_lock_face ]

Comment 9

[Darkspirit]

bp-9f1c8f76-b966-4278-81ad-ae99d0171031 31.10.17 03:21
[@ libfreetype.so.6.15.0@0x16729 | mozilla::gfx::Factory::ReleaseFTFace ]
> 35 	libxul.so 	mozilla::wr::Moz2DRenderCallback 		gfx/webrender_bindings/Moz2DImageRenderer.cpp:207
> 46 	libxul.so 	wr_moz2d_render_cb 				gfx/webrender_bindings/Moz2DImageRenderer.cpp:238
> 47 	libxul.so 	rayon_core::job::{{impl}}::execute<closure> 	gfx/webrender_bindings/src/moz2d_renderer.rs:138

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3563e187883
AddRef user data before passing to cairo_font_face_set_user_data. r=jrmuizel

Comment 11

[Eliza Balazs [:ebalazs_]]

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

Comment 12

[Darkspirit]

Nightly 58 x64 20171031220132 de_DE @ Debian Testing (KDE, Radeon RX480)
main profile: gpu process, layers forcel accel, webrender, blob-images, omtp, stylo-chrome
about:buildconfig > https://hg.mozilla.org/mozilla-central/rev/3f627210c55d6b2b0a0635683cc4a29102a90270

This build includes these three patches:
* comment 11 (AddRef user data before passing to cairo_font_face_set_user_data. r=jrmuizel)
* bug 1412565 comment 5 (Release the vec before deleting the item. r=lsalzman)
* bug 1380014 comment 19 (Fix up the webrender bindings. r=kats)
(Seen on https://hg.mozilla.org/mozilla-central/graph/3f627210c55d6b2b0a0635683cc4a29102a90270)

So far, I only got this crash:

Meldungs-ID 	Sendedatum
bp-c79532b8-9128-434b-a230-6c5030171101 01.11.17 14:20 [@ mozalloc_abort | abort | _cairo_array_allocate.cold.19 ]
(has webrender_bindings in the crash stack)

Comment 13

[Darkspirit]

Meldungs-ID 	Sendedatum
bp-5172a20a-1e15-4e4f-b9c0-4a4ca0171102 02.11.17 20:02
[@ libfreetype.so.6.15.0@0x14717 | mozilla::gfx::UnscaledFontFontconfig::CreateFromFontDescriptor ]

Comment 14

[Darkspirit]

build 2017-11-02_222620
bp-66880d1e-87eb-499e-8257-634ab0171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-02d7b1bc-9ab2-4a2c-9faa-2b5070171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-099b59bd-bdfe-4131-a935-c4d4f0171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-f0e69379-23b4-43cf-b272-490ef0171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-083e0efb-e4e4-4286-8ea0-0aad20171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-1139abc0-6e24-4936-88b9-c468c0171103	03.11.17 16:39 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]

---        crash fix bug 1412848 comment 6  @ 2017-11-03 10:05 +0000 ---
--- webrender update bug 1413178 comment 23 @ 2017-11-04 09:56 +0000 ---

build 2017-11-04_100412
bp-9e4e6185-670b-406b-b71d-c2c8d0171105 05.11.17 04:47 [@ _cairo_user_data_array_set_data.cold.16 ]
bp-1c0772fd-1270-4281-ad0c-534c20171105 05.11.17 04:25 [@ mozilla::gfx::ScaledFontFontconfig::CreateFromInstanceData ]
bp-4246c3ea-31a4-42bd-91da-a6c770171105 05.11.17 04:25 [@ _cairo_user_data_array_set_data.cold.16 ]

build 2017-11-05_100353 (https://hg.mozilla.org/mozilla-central/rev/e7fee7042d971d73c0e5caafba3b8d15da1bc8ca)
bp-c9033fe6-d4bd-45d8-ba1f-9270d0171105 05.11.17 20:05
[@ libfreetype.so.6.15.0@0x142c0 | libfreetype.so.6.15.0@0x1430b | libfreetype.so.6.15.0@0x2adb3f | libfreetype.so.6.15.0@0x1668d | libfreetype.so.6.15.0@0x167a1 | libfreetype.so.6.15.0@0x2adb3f | libfreetype.so.6.15.0@0x18368 | libfreetype.so.6.15.0@0x85... ]

Comment 15

[Lee Salzman [:lsalzman]]

Attachment: avoid race condition when setting Cairo ft font user data

So the relationship between bug 1380014 is clearer now. It caused UnscaledFonts to be shared between blob image threads, which thus means Cairo ft fonts may get shared between threads too.

This defeated an earlier workaround we used where each thread had its own isolated FT library, which as a consequence caused Cairo ft fonts to not be shared between threads, and thus avoid this issue.

Due to work on OMTP that locked down usage of FT library to be safe between threads, we no longer need the original hack.

All that is left is to now lock down setting the user data in NativeFontResourceFontconfig, so that multiple blob image threads no longer step on each other there.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b80f6e02f757
avoid race condition when setting Cairo ft font user data. r=jrmuizel

Comment 17

[Narcis Beleuzu [:NarcisB]]

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

Comment 18

[Darkspirit]

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #1)
> Nightly 58 x64 20171028100423 de_DE @ Debian Testing (KDE, Radeon RX480)
> FRESH PROFILE: layers.acceleration.force-enabled, gfx.webrender.enabled, gfx.webrender.blob-images
> 
> STR: Click on the library button very often. Maybe open one tab with bugzilla or about:preferences.
> Crash signatures differ. All but one crashes contain something from "webrender_bindings".

I can't reproduce my STR anymore.

layers.acceleration.force-enabled, gfx.webrender.enabled, gfx.webrender.blob-images
mozregression --bad 2017-11-06 --good 2017-11-09 --profile-persistence clone-first --find-fix
> 8:51.33 INFO: First good revision: 2e4bef09de27a8c6c92f027048ed33aa3322948e
> 8:51.33 INFO: Last bad revision: 62aeebcc676e93dc56a97d44753f4e2f963d43c3
> 8:51.33 INFO: Pushlog:
> https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=62aeebcc676e93dc56a97d44753f4e2f963d43c3&tochange=2e4bef09de27a8c6c92f027048ed33aa3322948e

I suspect this broke my STR:
> Lee Salzman — Bug 1403198 - support WR native font handles in blob image. r=jrmuizel
> Lee Salzman — Bug 1403198 - support WR font descriptors on Mac. r=jrmuizel
> Lee Salzman — Bug 1403198 - support WR font descriptors with DWrite. r=jrmuizel
> Lee Salzman — Bug 1403198 - support WR font descriptors with Fontconfig. r=jrmuizel
> Lee Salzman — Bug 1403198 - send font descriptors to WR instead of raw fonts where possible. r=jrmuizel
(landed on m-c on 2017-11-07 10:56)

Socorro: The last crash was with build 20171106100122 on 2017-11-07 10:58:34.

(In reply to Lee Salzman [:lsalzman] from comment #15)
Can this bug get marked as fixed?

Comment 19

[Lee Salzman [:lsalzman]]

Between bug 1403198 and the patch for the Cairo user data race, I believe this is fixed, yes.