Closed Bug 1485712 Opened Last year Closed Last year

SkTypeface memory leak in ScaledFonts with P-OMTP

Categories

(Core :: Graphics, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: lsalzman, Assigned: lsalzman)

References

(Blocks 2 open bugs)

Details

(Keywords: memory-leak, regression, Whiteboard: [gfx-noted])

Attachments

(1 file)

There is a race condition in ScaledFont::GetSkTypeface when multiple threads try to call GetSkTypeface at the same time. This bug unfortunately affects all platforms, not just Linux.

To work around this, I have made mTypeface an atomic pointer, and we use a compare-and-swap with null to verify we're actually the first one initializing this pointer. In the event that a thread loses, it just derefs/deallocates the typeface and all is well.

This strategy is lock-free and will avoid any extra locks/blocking so we shouldn't see added overhead from it when we go to render text.

I've restructured the code a bit to move the details of the atomic initialization into ScaledFontBase, so that each ScaledFont implementation does not have to reproduce it.
Attachment #9003502 - Flags: review?(rhunt)
Summary: SkTypeface memory eak in ScaledFonts with P-OMTP → SkTypeface memory leak in ScaledFonts with P-OMTP
Comment on attachment 9003502 [details] [diff] [review]
set SkTypeface atomically in ScaledFonts

Review of attachment 9003502 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch.
Attachment #9003502 - Flags: review?(rhunt) → review+
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/582994eb6fa9
set SkTypeface atomically in ScaledFonts. r=rhunt
https://hg.mozilla.org/mozilla-central/rev/582994eb6fa9
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.