Closed Bug 1187649 Opened 4 years ago Closed 4 years ago

[gtk3] various valgrind leaks

Categories

(Core :: Widget: Gtk, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

See https://treeherder.mozilla.org/logviewer.html#?job_id=9775519&repo=try

Some I don't know what they are, but most are rooted in widget code. Some are from code shared with gtk2, so that could indicate leaks in gtk itself. Or, since we have suppressions for gtk2, that could indicate leaks in our code that we're ignoring (see suppressions in build/valgrind/x86_64-redhat-linux-gnu.sup).

It seems to me we've been too prompt to add broad suppressions for gtk/fontconfig/pango/gobject "leaks" that may well root in our code.

One of the leaks in that try run roots in code specific to gtk3 on our end and *does* look like we are leaking something, but I don't know the gtk APIs well enough to tell whether that's true.

If we added similar broad suppressions for gtk3 libs, we'd probably hide real leaks.

Andrew, would you mind looking into this?
Sure, I'll take a look at this.
Assignee: nobody → acomminos
Let's separate those issues in two groups:
- leaks
- other errors

There are very many leaks, and fwiw, adding a broad suppression like we have for gtk2 gets rid of them all.
I'll file a separate bug for the latter, and those don't involve our code afaict.
Summary: [gtk3] various valgrind errors → [gtk3] various valgrind leaks
Filed bug 1187664
FWIW, here's a list of the suppressed leaks on Gtk+2 builds (grepping for used_suppression in a valgrind job log):
--23919-- used_suppression:     30 Bug 984196 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:80 suppressed: 47,791 bytes in 36 blocks
--23919-- used_suppression:    213 Bug 794373 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:46 suppressed: 21,980 bytes in 213 blocks
--23919-- used_suppression:      5 dl-hack3-cond-1 /usr/lib64/valgrind/default.supp:1206
--24032-- used_suppression:   5727 Bug 794373 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:46 suppressed: 692,901 bytes in 8,144 blocks
--24032-- used_suppression:    221 dl-hack3-cond-1 /usr/lib64/valgrind/default.supp:1206
--23915-- used_suppression:   1951 Bug 794366 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:32 suppressed: 378,290 bytes in 4,804 blocks
--23915-- used_suppression:   3605 Bug 794373 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:46 suppressed: 504,848 bytes in 5,292 blocks
--23915-- used_suppression:      1 Bug 979242 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:64 suppressed: 20,992 bytes in 1 blocks
--23915-- used_suppression:      3 Bug 794372 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:90 suppressed: 20,704 bytes in 647 blocks
--23915-- used_suppression:      2 Bug 794374 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:99 suppressed: 5,728 bytes in 179 blocks
--23915-- used_suppression:     74 Bug 793598 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:15 suppressed: 11,357 bytes in 90 blocks
--23915-- used_suppression:     30 Bug 794369 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:72 suppressed: 7,208 bytes in 53 blocks
--23915-- used_suppression:      3 Bug 794370 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:81 suppressed: 288 bytes in 6 blocks
--23915-- used_suppression:      1 PR_SetEnv requires its argument to be leaked, but does not appear on stacks. (See bug 793549.) /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:15 suppressed: 170 bytes in 1 blocks
--23915-- used_suppression:      3 Bug 794368 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:39 suppressed: 192 bytes in 3 blocks
--23915-- used_suppression:      1 PR_SetEnv requires its argument to be leaked, but does not appear on stacks. (See bug 793548.) /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:29 suppressed: 72 bytes in 1 blocks
--23915-- used_suppression:     73 Bug 793537 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/x86_64-redhat-linux-gnu.sup:8 suppressed: 2,880 bytes in 90 blocks
--23915-- used_suppression:      1 See bug 793535 /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:43 suppressed: 32 bytes in 1 blocks
--23915-- used_suppression:      9 PR_SetEnv requires its argument to be leaked, but does not appear on stacks. (See bug 793534.) /builds/slave/m-in-l64-valgrind-000000000000/src/build/valgrind/cross-architecture.sup:8 suppressed: 160 bytes in 9 blocks
--23915-- used_suppression:     29 dl-hack3-cond-1 /usr/lib64/valgrind/default.supp:1206

"Bug 794366" is suppressions of everything involving libgtk-x11
"Bug 794373" is suppressions of everything involving libgobject
(In reply to Mike Hommey [:glandium] from comment #4)
> FWIW, here's a list of the suppressed leaks on Gtk+2 builds (grepping for
> used_suppression in a valgrind job log)

... after the landing of bug 1187662
Most of these leaks appear to be internal to GTK/GDK/GLib/Pango/Cairo after some initial inspection. We are, however, definitely leaking a widget path- I'll post a patch to fix that.
Attachment #8639412 - Flags: review?(karlt) → review+
Keywords: leave-open
backed out for causing bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=12190147&repo=mozilla-inbound
Flags: needinfo?(acomminos)
Ah, sorry about that- forgot to put it in a MOZ_WIDGET_GTK == 3 block. Fixing.
Flags: needinfo?(acomminos)
Depends on: 1188780
This is what the leaks look like with debug symbols, and with the first patch from this bug applied.
https://treeherder.mozilla.org/logviewer.html#?job_id=9893633&repo=try

Looks like there are 3 big families:
- Fontconfig leaks. They all involve FcPatternObjectInsertElt and realloc. Here is an interesting comment related to these: https://code.google.com/p/skia/issues/detail?id=2879#c1 which leads to https://bugs.freedesktop.org/show_bug.cgi?id=8215 and https://bugs.freedesktop.org/show_bug.cgi?id=8428. I guess the problem is that valgrind is seeing them as lost when they are still referenced, only not by pointer. If pointers were used, valgrind would put them as still reachable, which is still a leak in some sense... anyways, clearly not our problem, and valid for a suppression.
- Gtk leaks. They all look like we are doing the right thing.
- Cairo leaks. They all are completely out of our reach. That is, we're not involved in dealing with cairo for those code paths, it's all in gtk.
I'm testing suppressions for Fontconfig and Cairo, but I'm not sure how to come up with suppressions for the Gtk leaks that wouldn't have been suppressed the real leak that was fixed by https://hg.mozilla.org/mozilla-central/rev/3bb4268b9651
Thank you for investigating these leaks so carefully, glandium.
Assignee: acomminos → mh+mozilla
Attachment #8640841 - Flags: review?(n.nethercote)
Comment on attachment 8640841 [details] [diff] [review]
Add valgrind suppressions for leaks in Gtk+3, cairo and fontconfig that are not due to Gecko

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

::: build/valgrind/x86_64-redhat-linux-gnu.sup
@@ +27,5 @@
>     fun:FcDefaultSubstitute
>     fun:_ZN17gfxPangoFontGroup11MakeFontSetEP14_PangoLanguagedP9nsAutoRefI10_FcPatternE
>     ...
>  }
> +# Fontconfig is going fancy is its cache structure and that confuses valgrind.

Nit: "with its"?
Attachment #8640841 - Flags: review?(n.nethercote) → review+
The cairo leaks at least may be due to not closing the display, which we don't do with early GTK3 versions because of a GTK bug.
Keywords: leave-open
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.