1779519 - Crash in [@ gfxFont::AddRef]

Status: RESOLVED FIXED

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

Description

[Andrew Osmond [:aosmond] (he/him)]

Crash report: https://crash-stats.mozilla.org/report/index/c81721aa-0f88-41ed-aff4-98ae50220713

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (Tried to remove an object that's not tracked)

Top 8 frames of crashing thread:

0 xul.dll gfxFont::AddRef gfx/thebes/gfxFont.h:1452
1 xul.dll gfxFontGroup::GetDefaultFont gfx/thebes/gfxTextRun.cpp:2166
2 xul.dll mozilla::dom::CanvasRenderingContext2D::DrawOrMeasureText dom/canvas/CanvasRenderingContext2D.cpp:4095
3 xul.dll mozilla::dom::CanvasRenderingContext2D::StrokeText dom/canvas/CanvasRenderingContext2D.cpp:3578
4 xul.dll mozilla::dom::OffscreenCanvasRenderingContext2D_Binding::strokeText dom/bindings/OffscreenCanvasRenderingContext2DBinding.cpp:3975
5 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3285
6 None @0x000003edaf882143 
7 xul.dll js::jit::EqualStringsHelperPure js/src/jit/VMFunctions.cpp:1583

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

The ExpirationTrackerImpl signature is for Android (but encompasses a lot more issues than just for OffscreenCanvas on DOM workers).

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1746110

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

We are holding the wrong lock in gfxFont::AddRef:
https://searchfox.org/mozilla-central/rev/a9e776193401900a8ceffa478ae6e9930e895ca1/gfx/thebes/gfxFont.h#1444

We should be holding:
https://searchfox.org/mozilla-central/rev/a9e776193401900a8ceffa478ae6e9930e895ca1/gfx/thebes/gfxFont.h#298

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1779519 - gfxFontCache expiration tracker operations should be more atomic.

The expiration state should not be protected by the gfxFont's mutex
since we don't hold it during most operations. Instead we should hold
gfxFontCache's mutex because then we can guarantee the operation is
atomic, particularly when a worker wants a font, and the main thread is
aging the generations.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4cd98b5984d
gfxFontCache expiration tracker operations should be more atomic. r=jfkthame

Comment 6

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing build bustages. CLOSED TREE
Backout link
Push with failures
Link to failure log
Failure line :
gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:669: Unified_cpp_windows_sdn0.obj] Error 1

Comment 7

[Andrew Osmond [:aosmond] (he/him)]

Missed some platform specific instances of the gfxFont already_AddRefed/RefPtr conversion.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a29ac3a0f194
gfxFontCache expiration tracker operations should be more atomic. r=jfkthame

Comment 9

[Cristian Tuns]

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