1431211 - blob images: fonts not removed from cache (leak) on bridge shutdown

Status: RESOLVED FIXED

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

Description

[Aria Beingessner [:Gankra]]

This appears to be the only remaining blocker for turning on blob-images on our CI.

This is very easy to reproduce. Just launch a browser session, go to a website, and close the window. At least on linux, UnscaledFontFontconfigs will be leaked.

This appears to be because they're stored in a static variable: the sFontDataTable.

Entries are only removed from this table by calling DeleteFontData, which webrender calls when it recieves the wr_resource_updates_delete_font message. This is normally called by WebRenderBridgeChild::RemoveExpiredFontKeys.

I *believe* the issue is that we're killing the BridgeChild before it has a chance to run RemoveExpiredFontKeys -- or rather the font keys aren't expiring before the last time we check.

I also believe it's not sufficient to just add a static cleanup of this cache, as this seems to be a legitimate leak that could grow in an unbounded manner if WRBridgeChild's are being created and deleted on a regular basis.

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: clean up WR blob image renderer resources on API destruction

This just crawls the sFontDataTable and discards any font keys that are in the WR API namespace that is just about to be destroyed. By this point we know no other things in the WR instance can render the key, so it is safe to just discard the keys before they would otherwise leak.

Comment 2

[Aria Beingessner [:Gankra]]

Comment on attachment 8943501 [details] [diff] [review]
clean up WR blob image renderer resources on API destruction

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

I'm not very familiar with this part of the codebase, but this seems reasonable from what I've read.

Comment 3

[Aria Beingessner [:Gankra]]

Might want to do a try build like the one in https://bugzilla.mozilla.org/show_bug.cgi?id=1362115#c8 to check that this solves our problem?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/294d12c7062b
clean up WR blob image renderer resources on API destruction. r=gankro

Comment 5

[Lee Salzman [:lsalzman]]

(In reply to Alexis Beingessner [:Gankro] from comment #3)
> Might want to do a try build like the one in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1362115#c8 to check that this
> solves our problem?

The leak is gone now, yes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0427966c52e94b61ed74e1b4cf899dc7818ead34

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/294d12c7062b