Bug 1965195

I don't think gtk_tree_view_set_cursor is implicated here; this is a leak in the a11y support.

Here is an upstream fix: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3660

(In reply to Matthias Clasen from comment #3)
> I don't think gtk_tree_view_set_cursor is implicated here; this is a leak in
> the a11y support.
> 
> Here is an upstream fix:
> https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3660

Hi,

I tried using the 2 commits listed in https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3660/commits, but I still see the leak.

$ git log
commit 27b3a31dcf66ef7e810945d9aba9dfd8cca017dc (HEAD -> gtk-3-22-jep2587)
Author: Matthias Clasen <mclasen>
Date:   Fri Jun 11 08:53:46 2021 -0400

    a11y: Fix ref counting in tree views
    
    GtkContainerCellAccessible wasn't unsetting accessible
    parents. Fix that.
    
    By itself, this doesn't help for freeing a memory leak,
    since AtkObject keeps a ref on its parent, so we never
    free the GtkContainerCellAccessible as long as it has children.

commit d023ec31aa3e2b01f12ad1b30c27694619884053
Author: Matthias Clasen <mclasen>
Date:   Fri Jun 11 08:55:48 2021 -0400

    a11y: Plug a memory leak with treeviews
    
    We need to explicitly remove the children from
    a GtkContainerCellAccessible, since they otherwise
    keep the parent alive.
    
    Fixes: #3981

$ git show 27b3a31dcf66ef7e810945d9aba9dfd8cca017dc
commit 27b3a31dcf66ef7e810945d9aba9dfd8cca017dc (HEAD -> gtk-3-22-jep2587)
Author: Matthias Clasen <mclasen>
Date:   Fri Jun 11 08:53:46 2021 -0400

    a11y: Fix ref counting in tree views
    
    GtkContainerCellAccessible wasn't unsetting accessible
    parents. Fix that.
    
    By itself, this doesn't help for freeing a memory leak,
    since AtkObject keeps a ref on its parent, so we never
    free the GtkContainerCellAccessible as long as it has children.

diff --git a/gtk/a11y/gtkcontainercellaccessible.c b/gtk/a11y/gtkcontainercellaccessible.c
index a756e3cadf..a40446fb47 100644
--- a/gtk/a11y/gtkcontainercellaccessible.c
+++ b/gtk/a11y/gtkcontainercellaccessible.c
@@ -30,12 +30,19 @@ struct _GtkContainerCellAccessiblePrivate
 G_DEFINE_TYPE_WITH_PRIVATE (GtkContainerCellAccessible, gtk_container_cell_accessible, GTK_TYPE_CELL_ACCESSIBLE)
 
 
+static void
+unset_child (gpointer child)
+{
+  atk_object_set_parent (ATK_OBJECT (child), NULL);
+  g_object_unref (child);
+}
+
 static void
 gtk_container_cell_accessible_finalize (GObject *obj)
 {
   GtkContainerCellAccessible *container = GTK_CONTAINER_CELL_ACCESSIBLE (obj);
 
-  g_list_free_full (container->priv->children, g_object_unref);
+  g_list_free_full (container->priv->children, unset_child);
 
   G_OBJECT_CLASS (gtk_container_cell_accessible_parent_class)->finalize (obj);
 }
@@ -157,6 +164,7 @@ gtk_container_cell_accessible_remove_child (GtkContainerCellAccessible *containe
   g_return_if_fail (GTK_IS_CELL_ACCESSIBLE (child));
   g_return_if_fail (container->priv->n_children > 0);
 
+  atk_object_set_parent (ATK_OBJECT (child), NULL);
   container->priv->children = g_list_remove (container->priv->children, child);
   container->priv->n_children--;
 
$ git show d023ec31aa3e2b01f12ad1b30c27694619884053
commit d023ec31aa3e2b01f12ad1b30c27694619884053
Author: Matthias Clasen <mclasen>
Date:   Fri Jun 11 08:55:48 2021 -0400

    a11y: Plug a memory leak with treeviews
    
    We need to explicitly remove the children from
    a GtkContainerCellAccessible, since they otherwise
    keep the parent alive.
    
    Fixes: #3981

diff --git a/gtk/a11y/gtktreeviewaccessible.c b/gtk/a11y/gtktreeviewaccessible.c
index efb9c9bc53..d3975d31b2 100644
--- a/gtk/a11y/gtktreeviewaccessible.c
+++ b/gtk/a11y/gtktreeviewaccessible.c
@@ -104,6 +104,17 @@ static void
 cell_info_free (GtkTreeViewAccessibleCellInfo *cell_info)
 {
   gtk_accessible_set_widget (GTK_ACCESSIBLE (cell_info->cell), NULL);
+  if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (cell_info->cell))
+    {
+      GList *children;
+
+      while ((children = gtk_container_cell_accessible_get_children (GTK_CONTAINER_CELL_ACCESSIBLE (cell_info->cell))) != NULL)
+        {
+          GtkCellAccessible *child = children->data;
+          gtk_container_cell_accessible_remove_child (GTK_CONTAINER_CELL_ACCESSIBLE (cell_info->cell), child);
+        }
+    }
+
   g_object_unref (cell_info->cell);
 
   g_free (cell_info);


I also added a g_print() to make sure I'm actually using the patched code:

$ git diff
diff --git a/gtk/a11y/gtktreeviewaccessible.c b/gtk/a11y/gtktreeviewaccessible.c
index d3975d31b2..baabe0c7db 100644
--- a/gtk/a11y/gtktreeviewaccessible.c
+++ b/gtk/a11y/gtktreeviewaccessible.c
@@ -103,6 +103,7 @@ gtk_tree_view_accessible_get_data_quark (void)
 static void
 cell_info_free (GtkTreeViewAccessibleCellInfo *cell_info)
 {
+  g_print("cell_info_free() called\n");
   gtk_accessible_set_widget (GTK_ACCESSIBLE (cell_info->cell), NULL);
   if (GTK_IS_CONTAINER_CELL_ACCESSIBLE (cell_info->cell))
     {

I do see the output from this line when I run the snippet from the description. However jemalloc still reports a memory leak:

(n = 5 in the snippet)
[1920 bytes leaked]
je_prof_backtrace (/home/sandreev/git/misc/jemalloc/src/prof.c:636 (discriminator 2))
je_malloc_default (/home/sandreev/git/misc/jemalloc/src/jemalloc.c:2289)
g_malloc (??:?)
g_slice_alloc (??:?)
g_slice_alloc0 (??:?)
g_type_create_instance (??:?)
g_object_unref (??:?)
g_object_new_valist (??:?)
g_object_new (??:?)
gtk_renderer_cell_accessible_new (/home/sandreev/git/gtk/gtk/gtk/a11y/gtkrenderercellaccessible.c:116)
create_cell (/home/sandreev/git/gtk/gtk/gtk/a11y/gtktreeviewaccessible.c:436)
_gtk_tree_view_accessible_add_state (/home/sandreev/git/gtk/gtk/gtk/a11y/gtktreeviewaccessible.c:1996)
gtk_tree_view_real_set_cursor (/home/sandreev/git/gtk/gtk/gtk/gtktreeview.c:13313)
gtk_tree_view_set_cursor_on_cell (/home/sandreev/git/gtk/gtk/gtk/gtktreeview.c:13440)
?? (??:0)
__libc_start_main (/usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274)
?? (??:0)

(n = 11 in the snippet)
[4224 bytes leaked]
je_prof_backtrace (/home/sandreev/git/misc/jemalloc/src/prof.c:636 (discriminator 2))
je_malloc_default (/home/sandreev/git/misc/jemalloc/src/jemalloc.c:2289)
g_malloc (??:?)
g_slice_alloc (??:?)
g_slice_alloc0 (??:?)
g_type_create_instance (??:?)
g_object_unref (??:?)
g_object_new_valist (??:?)
g_object_new (??:?)
gtk_renderer_cell_accessible_new (/home/sandreev/git/gtk/gtk/gtk/a11y/gtkrenderercellaccessible.c:116)
create_cell (/home/sandreev/git/gtk/gtk/gtk/a11y/gtktreeviewaccessible.c:436)
_gtk_tree_view_accessible_add_state (/home/sandreev/git/gtk/gtk/gtk/a11y/gtktreeviewaccessible.c:1996)
gtk_tree_view_real_set_cursor (/home/sandreev/git/gtk/gtk/gtk/gtktreeview.c:13313)
gtk_tree_view_set_cursor_on_cell (/home/sandreev/git/gtk/gtk/gtk/gtktreeview.c:13440)
?? (??:0)
__libc_start_main (/usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:274)
?? (??:0)

Same if I run the SWT snippet here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=573758#c2

Did I make a mistake somewhere, or are more patches on the way (or are the proposed patches unfinished)?

Best regards,
Simeon

See discussion onhttps://bugs.eclipse.org/bugs/show_bug.cgi?id=573758, apparently the patches work on GTK 3.24.x but, unfortunately, not on GTK 3.22.x. We'll need a fix that works on GTK 3.22.x, as RHEL 7 is on GTK 3.22.x.

Thanks for testing, Simeon.

We'll have to look at the delta between 3.22.30 and 3.24.x to see what additional changes we need.

My testing on the 3.22 branch shows the fix working as I would expect - all the container cell accessibles that are created get finalized when I destroy the window containing the treeview.

Is it possible that you only used one of the two patches?

I'll add prints to both touched sources (so far I had print sat only one location) and will do a clean compile. Maybe my LD_LIBRARY_PATH is not including all changed .so files.

OK, I added prints to the other changed source and did a "make clean; make" (I see all expected prints, so the code does run with the changes). I don't see the growing leak now. Neither with the SWT snippet from https://bugs.eclipse.org/bugs/show_bug.cgi?id=573758#c2, nor with the GTK+ snippet from the description here.

Sorry for the noise, likely I messed up the compile step. Thanks for the fix!

Hi,
I tried to reproduce the leak:
1. installed:
gtk3-3.22.30-3.el7.x86_64
gtk3-devel-3.22.30-3.el7.x86_64

2. run:
gcc -g table.c  `pkg-config --cflags --libs gtk+-3.0`  -o table

3. then:
valgrind -s --leak-check=full ./table

I closed table app and got leak summary:
LEAK SUMMARY:
==2510==    definitely lost: 2,800 bytes in 9 blocks
==2510==    indirectly lost: 23,566 bytes in 788 blocks
==2510==      possibly lost: 5,412 bytes in 56 blocks
==2510==    still reachable: 1,978,213 bytes in 24,350 blocks
==2510==                       of which reachable via heuristic:
==2510==                         length64           : 7,632 bytes in 120 blocks
==2510==                         newarray           : 2,272 bytes in 62 blocks
==2510==         suppressed: 0 bytes in 0 blocks
==2510== 
==2510== ERROR SUMMARY: 42 errors from 42 contexts (suppressed: 0 from 0)


Then I took the newest version (gtk3-3.22.30-7.el7_9) for verification and did the same procedure, but I get at the bottom of the output:
==2640== LEAK SUMMARY:
==2640==    definitely lost: 3,680 bytes in 14 blocks
==2640==    indirectly lost: 21,366 bytes in 753 blocks
==2640==      possibly lost: 5,412 bytes in 56 blocks
==2640==    still reachable: 1,977,709 bytes in 24,336 blocks
==2640==                       of which reachable via heuristic:
==2640==                         length64           : 7,632 bytes in 120 blocks
==2640==                         newarray           : 2,272 bytes in 62 blocks
==2640==         suppressed: 0 bytes in 0 blocks
==2640== 
==2640== ERROR SUMMARY: 42 errors from 42 contexts (suppressed: 0 from 0)

so it seems the leak is there and is bigger in the new version (gtk3-3.22.30-7.el7_9).
Am I missing something?

(In reply to Radek Duda from comment #14)
> Hi,
> I tried to reproduce the leak:
> 1. installed:
> gtk3-3.22.30-3.el7.x86_64
> gtk3-devel-3.22.30-3.el7.x86_64
> 
> 2. run:
> gcc -g table.c  `pkg-config --cflags --libs gtk+-3.0`  -o table
> 
> 3. then:
> valgrind -s --leak-check=full ./table
> 
> I closed table app and got leak summary:
> LEAK SUMMARY:
> ==2510==    definitely lost: 2,800 bytes in 9 blocks
> ==2510==    indirectly lost: 23,566 bytes in 788 blocks
> ==2510==      possibly lost: 5,412 bytes in 56 blocks
> ==2510==    still reachable: 1,978,213 bytes in 24,350 blocks
> ==2510==                       of which reachable via heuristic:
> ==2510==                         length64           : 7,632 bytes in 120
> blocks
> ==2510==                         newarray           : 2,272 bytes in 62
> blocks
> ==2510==         suppressed: 0 bytes in 0 blocks
> ==2510== 
> ==2510== ERROR SUMMARY: 42 errors from 42 contexts (suppressed: 0 from 0)
> 
> 
> Then I took the newest version (gtk3-3.22.30-7.el7_9) for verification and
> did the same procedure, but I get at the bottom of the output:
> ==2640== LEAK SUMMARY:
> ==2640==    definitely lost: 3,680 bytes in 14 blocks
> ==2640==    indirectly lost: 21,366 bytes in 753 blocks
> ==2640==      possibly lost: 5,412 bytes in 56 blocks
> ==2640==    still reachable: 1,977,709 bytes in 24,336 blocks
> ==2640==                       of which reachable via heuristic:
> ==2640==                         length64           : 7,632 bytes in 120
> blocks
> ==2640==                         newarray           : 2,272 bytes in 62
> blocks
> ==2640==         suppressed: 0 bytes in 0 blocks
> ==2640== 
> ==2640== ERROR SUMMARY: 42 errors from 42 contexts (suppressed: 0 from 0)
> 
> so it seems the leak is there and is bigger in the new version
> (gtk3-3.22.30-7.el7_9).
> Am I missing something?

What does the detailed leak report say? Maybe there is a new leak.

It is in the attachment

I found there e.g.:
==3150== 7,325 (1,120 direct, 6,205 indirect) bytes in 10 blocks are definitely lost in loss record 8,671 of 8,716
==3150==    at 0x6BDC6AA: g_type_create_instance (gtype.c:1846)
==3150==    by 0x6BC028C: g_object_new_internal (gobject.c:1799)
==3150==    by 0x6BC21B0: g_object_new_valist (gobject.c:2122)
==3150==    by 0x6BC24F8: g_object_new (gobject.c:1642)
==3150==    by 0x4ED440D: gtk_renderer_cell_accessible_new (gtkrenderercellaccessible.c:116)
==3150==    by 0x4EDC4C7: create_cell_accessible_for_renderer (gtktreeviewaccessible.c:380)
==3150==    by 0x4EDC4C7: create_cell_accessible (gtktreeviewaccessible.c:414)
==3150==    by 0x4EDC4C7: create_cell (gtktreeviewaccessible.c:435)
==3150==    by 0x4EDE948: _gtk_tree_view_accessible_add_state (gtktreeviewaccessible.c:1995)
==3150==    by 0x5199D21: gtk_tree_view_real_set_cursor (gtktreeview.c:13313)
==3150==    by 0x519D6FB: gtk_tree_view_set_cursor_on_cell (gtktreeview.c:13440)
==3150==    by 0x401670: main (table.c:75)
==3150==

Is it possible the fix didn't get merged to 3.22 stream? I pulled the gtk-3-22 branch (remote https://github.com/GNOME/gtk), I don't see the commits with the fixes.

(In reply to Simeon Andreev from comment #19)
> Is it possible the fix didn't get merged to 3.22 stream? I pulled the
> gtk-3-22 branch (remote https://github.com/GNOME/gtk), I don't see the
> commits with the fixes.

No further development happens on the gtk-3-22 branch upstream, but the proposed change is to cherry-pick the 2 patches mentioned in comment #4, which apply cleanly to 3.22.30.

(In reply to David King from comment #20)
> (In reply to Simeon Andreev from comment #19)
> > Is it possible the fix didn't get merged to 3.22 stream? I pulled the
> > gtk-3-22 branch (remote https://github.com/GNOME/gtk), I don't see the
> > commits with the fixes.
> 
> No further development happens on the gtk-3-22 branch upstream, but the
> proposed change is to cherry-pick the 2 patches mentioned in comment #4,
> which apply cleanly to 3.22.30.

cherry-pick to where?

I'm not sure I understand this then, in combination with comment 14:

>  David King 2022-01-18 13:11:16 UTC
> Status: POST → MODIFIED
> Fixed In Version: gtk3-3.22.30-7.el7_9


> so it seems the leak is there and is bigger in the new version (gtk3-3.22.30-7.el7_9).

Please clarify which GTK3 versions are supposed to have the fix.

(In reply to Simeon Andreev from comment #21)
> (In reply to David King from comment #20)
> > (In reply to Simeon Andreev from comment #19)
> > > Is it possible the fix didn't get merged to 3.22 stream? I pulled the
> > > gtk-3-22 branch (remote https://github.com/GNOME/gtk), I don't see the
> > > commits with the fixes.
> > 
> > No further development happens on the gtk-3-22 branch upstream, but the
> > proposed change is to cherry-pick the 2 patches mentioned in comment #4,
> > which apply cleanly to 3.22.30.
> 
> cherry-pick to where?

To the gtk3 package on RHEL 7.9.

> I'm not sure I understand this then, in combination with comment 14:
> 
> >  David King 2022-01-18 13:11:16 UTC
> > Status: POST → MODIFIED
> > Fixed In Version: gtk3-3.22.30-7.el7_9
> 
> 
> > so it seems the leak is there and is bigger in the new version (gtk3-3.22.30-7.el7_9).
> 
> Please clarify which GTK3 versions are supposed to have the fix.

The future, as-yet-unreleased version of the gtk3 package for RHEL 7.9.

I saw the patches were applied accordingly, nevertheless memory leak is still there. Thus moving back to ASSIGNED state.

Upstream fix is here: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/4471

David, can you get that into our rhel builds? Thanks

I run
$ valgrind --leak-check=full --log-file=out  --suppressions=/home/test/gtk.supp ./table
with gtk3-3.22.30-8.el7_9.x86_64
There is is still a small leak, but seems not to be a11y leak:

==2866== 13,406 (1,024 direct, 12,382 indirect) bytes in 2 blocks are definitely lost in loss record 8,806 of 8,824
==2866==    at 0x4C2C291: realloc (vg_replace_malloc.c:836)
==2866==    by 0x89F3059: FcPatternObjectInsertElt (fcpat.c:526)
==2866==    by 0x89F3844: FcPatternObjectAddWithBinding (fcpat.c:720)
==2866==    by 0x89F3CBF: FcPatternObjectAdd (fcpat.c:749)
==2866==    by 0x89F3CBF: FcPatternObjectAddBool (fcpat.c:895)
==2866==    by 0x89F3CBF: FcPatternAddBool (fcpat.c:901)
==2866==    by 0x69431A7: _cairo_ft_font_options_substitute (cairo-ft-font.c:3433)
==2866==    by 0x5FF8A9F: pango_cairo_fc_font_map_fontset_key_substitute (pangocairo-fcfontmap.c:119)
==2866==    by 0x87C8DA3: pango_fc_font_map_get_patterns (pangofc-fontmap.c:1664)
==2866==    by 0x87C8DA3: pango_fc_font_map_load_fontset (pangofc-fontmap.c:1768)
==2866==    by 0x18D2CAE2: ??? (in /usr/lib64/gtk-3.0/modules/libpk-gtk-module.so)
==2866==    by 0x664798F: itemize_state_update_for_new_run (pango-context.c:1397)
==2866==    by 0x664798F: itemize_state_process_run (pango-context.c:1436)
==2866==    by 0x6648C77: pango_itemize_with_base_dir (pango-context.c:1583)
==2866==    by 0x665059F: pango_layout_check_lines (pango-layout.c:3953)
==2866==    by 0x66525D9: pango_layout_get_extents_internal (pango-layout.c:2536)
==2866==
==2866== LEAK SUMMARY:
==2866==    definitely lost: 1,024 bytes in 2 blocks
==2866==    indirectly lost: 15,161 bytes in 648 blocks
==2866==      possibly lost: 4,372 bytes in 36 blocks
==2866==    still reachable: 1,981,552 bytes in 24,462 blocks
==2866==                       of which reachable via heuristic:
==2866==                         length64           : 7,632 bytes in 120 blocks
==2866==                         newarray           : 2,272 bytes in 62 blocks
==2866==         suppressed: 2,576 bytes in 22 blocks
==2866== Reachable blocks (those to which a pointer was found) are not shown.
==2866== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2866==
==2866== For lists of detected and suppressed errors, rerun with: -s
==2866== ERROR SUMMARY: 35 errors from 35 contexts (suppressed: 6 from 6)

is this expected Matthias?

Yeah, that is unrelated to the leaks this bug was about. I believe it is not a leak (valgrind can't understand the way fontconfig handles memory), but just something that slips through our current suppressions.

I thought, but just to be sure .. thanks Matthias.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (gtk3 bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:1192