Closed Bug 1683878 Opened 4 years ago Closed 4 years ago

nsLookAndFeel::NativeGetFont leaks when called on rayon threads

Categories

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

Desktop
macOS
defect

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox86 --- fixed

People

(Reporter: jrmuizel, Assigned: jfkthame)

References

Details

Attachments

(1 file)

objc[87589]: MISSING POOLS: (0x700010356000) Object 0x13b214d20 of class NSCTFont autoreleased with no pool in place - just leaking - break on objc_autoreleaseNoPool() to debug
Process 87589 stopped
* thread #96, name = 'StyleThread#4', stop reason = breakpoint 1.1
    frame #0: 0x00007fff6e10168c libobjc.A.dylib`objc_autoreleaseNoPool
libobjc.A.dylib`objc_autoreleaseNoPool:
->  0x7fff6e10168c <+0>: retq   
    0x7fff6e10168d <+1>: nop    

libobjc.A.dylib`AutoreleasePoolPage::busted<void (*)(char const*, ...)>:
    0x7fff6e10168e <+0>: pushq  %rbp
    0x7fff6e10168f <+1>: movq   %rsp, %rbp
Target 0: (firefox) stopped.
(lldb) bt
* thread #96, name = 'StyleThread#4', stop reason = breakpoint 1.1
  * frame #0: 0x00007fff6e10168c libobjc.A.dylib`objc_autoreleaseNoPool
    frame #1: 0x00007fff6e10168a libobjc.A.dylib`AutoreleasePoolPage::autoreleaseNoPage(objc_object*) + 282
    frame #2: 0x00007fff6e0e8636 libobjc.A.dylib`objc_object::rootAutorelease2() + 32
    frame #3: 0x00007fff66297108 UIFoundation`__NSGetMetaFontInstanceWithType + 270
    frame #4: 0x0000000102b3869e XUL`gfxMacPlatformFontList::LookupSystemFont(this=<unavailable>, aSystemFontID=<unavailable>, aSystemFontName="", aFontStyle=0x0000700010353dc8) at gfxMacPlatformFontList.mm:0 [opt]
    frame #5: 0x000000010471f083 XUL`nsLookAndFeel::NativeGetFont(this=<unavailable>, aID=<unavailable>, aFontName=0x0000700010353e78, aFontStyle=<unavailable>) at nsLookAndFeel.mm:644:3 [opt]
    frame #6: 0x00000001049358e5 XUL`nsLayoutUtils::ComputeSystemFont(aSystemFont=0x0000700010353fa0, aFontID=PullDownMenu, aDefaultVariableFont=0x000000011571e818, aDocument=0x000000011d5d6000) at nsLayoutUtils.cpp:9300:7 [opt]
    frame #7: 0x000000010483f881 XUL`::Gecko_nsFont_InitSystem(aDest=0x0000700010353fa0, aFontId=14, aFont=<unavailable>, aDocument=0x000000011d5d6000) at GeckoBindings.cpp:1009:3 [opt]
    frame #8: 0x0000000107979e5d XUL`_$LT$style..properties..longhands..system_font..SystemFont$u20$as$u20$style..values..computed..ToComputedValue$GT$::to_computed_value::hc834f65ad7321933(self=<unavailable>, cx=0x00007000103546f0) at font.rs:3448:21 [opt]
    frame #9: 0x000000010785bb28 XUL`style::properties::longhands::system_font::resolve_system_font::h00c5114386296125(system=MozPullDownMenu, context=0x00007000103546f0) at font.rs:3529:32 [opt]
    frame #10: 0x0000000107859ee0 XUL`style::properties::longhands::font_family::cascade_property::h9ceb9b872293ca97(declaration=<unavailable>, context=0x00007000103546f0) at font.rs:104:21 [opt]
    frame #11: 0x000000010773c768 XUL`style::properties::cascade::Cascade::apply_properties::h3c969f234960e44e at cascade.rs:556:9 [opt]
    frame #12: 0x000000010773c756 XUL`style::properties::cascade::Cascade::apply_properties::h3c969f234960e44e(self=0x0000700010354338, apply_reset=<unavailable>, declarations=<unavailable>) at cascade.rs:673 [opt]
    frame #13: 0x000000010773bc0a XUL`style::properties::cascade::cascade_rules::h8dfaba9db86337ac at cascade.rs:329:9 [opt]
Severity: -- → S3
OS: Unspecified → macOS
Priority: -- → P3
Hardware: Unspecified → Desktop
Flags: needinfo?(jfkthame)

Jonathan, it seems like gfxMacPlatformFontList::LookupSystemFont is called a lot. Should we be caching the results some how?

Yeah, probably we should. I see the Windows implementation of nsLookAndFeel::NativeGetFont uses a caching scheme[1]; probably we should do something similar here (or maybe it should be higher up in platform-independent code).

We also need a local autorelease pool, by the looks of it. Pools are per-thread, so the main thread's pool doesn't catch things here when called from stylo threads.

[1] A bit worryingly, it doesn't look like the cache in NativeGetFont on Windows is thread-safe. So is it OK for stylo to be calling this?

Flags: needinfo?(jfkthame)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED

Hmm, it's unclear to me whether caching here would gain us much; I'm not seeing LookupSystemFont showing up in a quick look at a few profiles. But if we do see cases where it's a hotspot, we could certainly consider it. Did you have an example where it seems to be a bottleneck?

Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/056107977dd2 Use a local autorelease pool in gfxMacPlatformFontList::LookupSystemFont because it may be used off-main-thread. r=jrmuizel
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: