Debug Compilation fails with Pango 1.44
Categories
(Core :: Graphics: Text, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: u480271, Assigned: jfkthame)
References
Details
Attachments
(1 file)
pango_fc_font_map_shutdown was removed in the transition from 1.43.0 to 1.44.0. This code is used to avoid false reporting of leaked objects. https://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#2750
David, do you think we still require this code? I'm wondering if it could be removed?|
At the time I added it, it did make shutdown substantially cleaner in terms of leak reports, which made it easier to find real leaks. I haven't looked into what changed in the pango code recently, though, so it's hard to know what changed, or how it shuts down now.
|
||
Updated•5 years ago
|
|
Assignee | |
Comment 4•5 years ago
|
||
(In reply to David Baron :dbaron: 🏴 ⌚UTC-7 from comment #2)
At the time I added it, it did make shutdown substantially cleaner in terms of leak reports, which made it easier to find real leaks. I haven't looked into what changed in the pango code recently, though, so it's hard to know what changed, or how it shuts down now.
Aside from whatever has changed within the pango code, the other thing that's changed since then is that we no longer use pango for any font-related work (font selection, shaping, etc) in thebes. I assume any native GTK widgets we use may still be relying on pango internally, but we don't use it directly.
(There is one remaining pango use that I can think of within the Gecko code, in NS_GetComplexLineBreaks, but that doesn't involve working with fonts at all.)
To see what would happen, I tried a debug build with this removed, and so far tryserver hasn't reported any apparent leaks, though not all results are in yet: https://treeherder.mozilla.org/#/jobs?repo=try&revision=17095a21bf8582f588b55fbec56f285ade7938eb.
|
|
Assignee | |
Comment 5•5 years ago
|
||
I would note that the leaks that this was cleaning up are not leaks that would be reported by any of the leak tests we run in CI.
... in particular, at the time I was using trace-malloc, which has since been removed. I'm told DMD can do what it did, but I haven't tried it. It's likely valgrind leak reporting also could, at least if you disable the suppressions and tell it to dump all leaked objects at shutdown.
|
|
Assignee | |
Comment 8•5 years ago
|
||
OK, that's worth noting, thanks.
We could keep this in builds created with older pango versions for now, using an #ifdef (as per https://phabricator.services.mozilla.com/D42127#1271969 and following), but that introduces the risk that such builds may eventually fail on a system with a newer library. And in any case at some point our build systems will no doubt end up with a new pango -- maybe not for some time, as we don't seem to update frequently, but it'll come -- and then we'll lose this even if we #ifdef it for now.
On balance, I'm inclined to just go ahead with the removal; then if/when a situation arises where we're wanting to go leak-chasing in a similar way, we'll need to look at the current state of pango and decide how best to handle it. Maybe with pango 1.44.x we could use dlsym() to access the function even without the header file, as long as it continues to exist with the same signature. But I don't feel it's worth pursuing that for now, unless someone is actively depending on this.
Pushed by jkew@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c670f40348ee Remove call to pango_fc_font_map_shutdown, which was moved out of public headers in Pango 1.44. r=dbaron
|
||
Comment 10•5 years ago
|
||
| bugherder | ||
|
||
Updated•5 years ago
|
Description
•