RESOLVED INVALID261274
[FreeType][GTK] leaking memory with cairo
https://bugs.webkit.org/show_bug.cgi?id=261274
Summary [FreeType][GTK] leaking memory with cairo
Christian Hesse
Reported 2023-09-07 06:36:41 PDT
Calling `cairo_font_options_create()` returns a structure that contains an allocated property [0]. That used to be no problem when cairo font options were initialized just once, however this changed with commit 4fac13b14ab43258032797d51b868da33064af9e ("Network process shouldn't load Cairo ") [1] where the function is called when ever the data is used. Now the allocated property leaks, filling up all RAM over time. [0] https://gitlab.freedesktop.org/cairo/cairo/-/blob/master/src/cairo-font-options.c?ref_type=heads#L97 [1] https://github.com/WebKit/WebKit/commit/4fac13b14ab43258032797d51b868da33064af9e
Attachments
source code of reproducing application (2.39 KB, text/plain)
2023-09-07 07:14 PDT, Christian Hesse
no flags
missif log file (379.11 KB, text/plain)
2023-09-07 07:14 PDT, Christian Hesse
no flags
Christian Hesse
Comment 1 2023-09-07 07:12:38 PDT
Doing some more testing... I am not even sure my analysis is correct. :-/ Let me recap: I am running Arch Linux, and my application started leaking memory after the update from webkit2gtk 2.38.5-1 (ok) to 2.40.0-2 (bad). I will attach my application source code and a massif log file which was generated with: valgrind --tool=massif --trace-children=yes ./browse -u https://eworm.de/tmp/webkit.html
Christian Hesse
Comment 2 2023-09-07 07:14:10 PDT
Created attachment 467584 [details] source code of reproducing application
Christian Hesse
Comment 3 2023-09-07 07:14:28 PDT
Created attachment 467585 [details] missif log file
Fujii Hironori
Comment 4 2023-09-07 13:47:28 PDT
Weird. 261107@main seems to be innocent. The change doesn't create cairo_font_options repeatedly. Did you confirm if your problem was fixed by reverting the change?
Christian Hesse
Comment 5 2023-09-08 02:50:20 PDT
You are right... It is not caused by 261107@main. After my initial doubt I just reverted the commit, and the issue still exists. So anything I can tell is that it was fine with 2.38.5-1, was introduced with 2.40.0-2 and is still there with 2.40.5. Any idea?
Ahmad Saleem
Comment 6 2023-09-08 02:52:22 PDT
Almost 5000+ commit between this range (using GitHub tags): https://github.com/WebKit/WebKit/compare/webkitgtk-2.38.1...webkitgtk-2.40.2
Christian Hesse
Comment 7 2023-09-08 03:12:34 PDT
Yes, that's the problem... (Btw, this is about 2.38.5 and 2.40.0, the appended number with dash is Arch Linux incremental package release). I had thought about bisecting. Actually that failed, as I was suffering build issues. To date I can not even build 2.38.5 because of toolchain updates.
Christian Hesse
Comment 8 2023-09-12 16:50:00 PDT
I managed to finish a bisect... Took me a lot of time and did cost some extra grey hair. 🤪 c0289be0b9ab80469106e8b06cf75b9c62f37ec9 is the first bad commit commit c0289be0b9ab80469106e8b06cf75b9c62f37ec9 Author: Vitaly Dyachkov <vitaly@igalia.com> Date: Mon Mar 13 02:50:36 2023 -0700 Cherry-pick 261566@main (2d385163e7ce). https://bugs.webkit.org/show_bug.cgi?id=177039 [Freetype] Apply basic font properties as font variation settings https://bugs.webkit.org/show_bug.cgi?id=177039 Reviewed by Carlos Garcia Campos. This patch makes sure that the basic font properties, such as `font-weight`, `font-width`, and `font-style`, are applied in the same way as `font-variation-settings`. * LayoutTests/fast/text/variations/basic-properties-expected.html: Added. * LayoutTests/fast/text/variations/basic-properties.html: Added. * LayoutTests/fast/text/variations/resources/AmstelvarAlpha-VF.woff2: Added. * LayoutTests/fast/text/variations/resources/Roboto-VF.woff2: Added. * LayoutTests/fast/text/variations/resources/jost-VF.woff2: Added. * Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp: (WebCore::buildVariationSettings): Canonical link: https://commits.webkit.org/261566@main .../text/variations/basic-properties-expected.html | 33 ++++++++++++++++++++ .../fast/text/variations/basic-properties.html | 34 +++++++++++++++++++++ .../variations/resources/AmstelvarAlpha-VF.woff2 | Bin 0 -> 76720 bytes .../fast/text/variations/resources/Roboto-VF.woff2 | Bin 0 -> 1475864 bytes .../fast/text/variations/resources/jost-VF.woff2 | Bin 0 -> 141636 bytes .../graphics/freetype/FontCacheFreeType.cpp | 25 +++++++++++++-- .../platform/graphics/freetype/FontCacheFreeType.h | 2 +- .../freetype/FontCustomPlatformDataFreeType.cpp | 2 +- 8 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 LayoutTests/fast/text/variations/basic-properties-expected.html create mode 100644 LayoutTests/fast/text/variations/basic-properties.html create mode 100644 LayoutTests/fast/text/variations/resources/AmstelvarAlpha-VF.woff2 create mode 100644 LayoutTests/fast/text/variations/resources/Roboto-VF.woff2 create mode 100644 LayoutTests/fast/text/variations/resources/jost-VF.woff2
Christian Hesse
Comment 9 2023-09-21 00:26:26 PDT
All of the leaks happened in cairo, and issues are addressed there. Closing as invalid.
Note You need to log in before you can comment on or make changes to this bug.