After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 774185 - GtkPlacesSidebar does not unref itsel as many times as it references
GtkPlacesSidebar does not unref itsel as many times as it references
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-11-10 08:50 UTC by Massimo
Modified: 2016-11-10 20:17 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Massimo 2016-11-10 08:50:14 UTC
After setting GOBJECT_DEBUG=objects and running the following program
(Canceling the dialog after having clicked on few different rows in the
places sidebar (not in the bookmarks section))

#include <gtk/gtk.h>

int
main (int   argc,
      char *argv[])
{
  if (gtk_init_check (&argc, &argv))
    for (gint i = 0; i < ITERATIONS; ++i)
      {
        GtkWidget *dialog = gtk_file_chooser_dialog_new ("Open File",
                                                         NULL,
                                                         GTK_FILE_CHOOSER_ACTION_OPEN,
                                                         "Cancel", GTK_RESPONSE_CANCEL,
                                                         NULL);
        gtk_dialog_run (GTK_DIALOG (dialog));

        gtk_widget_destroy (dialog);
      } 
      
  return 0;
}


The GtkPlacesSidebar object alive at exit has a reference count
equal to the number of different rows clicked.

The suspicious code is here:

https://git.gnome.org/browse/gtk+/tree/gtk/gtkplacessidebar.c?h=gtk-3-22#n3512

the early return FALSE code path skips the g_object_unref
at the end of the function.
Comment 1 Massimo 2016-11-10 12:25:54 UTC
Correct program.

#include <gtk/gtk.h>

int
main (int   argc,
      char *argv[])
{
  if (gtk_init_check (&argc, &argv))
    {
      GtkWidget *dialog = gtk_file_chooser_dialog_new ("Open File",
                                                       NULL,
                                                       GTK_FILE_CHOOSER_ACTION_OPEN,
                                                       "Cancel", GTK_RESPONSE_CANCEL,
                                                       NULL);
      gtk_dialog_run (GTK_DIALOG (dialog));

      gtk_widget_destroy (dialog);
    }

  return 0;
}


There is also a leak in gtk/a11y/gtktreeviewaccessible.c that
can be shown with the above program. You need to click in the
main file chooser treeview on different file names and in the
output there are as many GtkContainerCellAccessible instances 
as clicks.

I think that when cell_info->cell is a GtkContainerCellAccessible
instance cell_info_free

https://git.gnome.org/browse/gtk+/tree/gtk/a11y/gtktreeviewaccessible.c?h=gtk-3-22#n104

should call gtk_container_cell_accessible_remove_child and 
g_object_unref as many times as gtk_container_cell_accessible_add_child
and create_cell_accessible_for_render were called in 
create_cell_accessible:

https://git.gnome.org/browse/gtk+/tree/gtk/a11y/gtktreeviewaccessible.c?h=gtk-3-22#n404
Comment 2 Massimo 2016-11-10 12:37:34 UTC
(In reply to Massimo from comment #1)
> Correct program.
> 
> #include <gtk/gtk.h>
> 
> int
> main (int   argc,
>       char *argv[])
> {
>   if (gtk_init_check (&argc, &argv))
>     {
>       GtkWidget *dialog = gtk_file_chooser_dialog_new ("Open File",
>                                                        NULL,
>                                                       
> GTK_FILE_CHOOSER_ACTION_OPEN,
>                                                        "Cancel",
> GTK_RESPONSE_CANCEL,
>                                                        NULL);
>       gtk_dialog_run (GTK_DIALOG (dialog));
> 
>       gtk_widget_destroy (dialog);
>     }
> 
>   return 0;
> }
> 
> 
> There is also a leak in gtk/a11y/gtktreeviewaccessible.c that
> can be shown with the above program. You need to click in the
> main file chooser treeview on different file names and in the
> output there are as many GtkContainerCellAccessible instances 
> as clicks.
> 
> I think that when cell_info->cell is a GtkContainerCellAccessible
> instance cell_info_free
> 
> https://git.gnome.org/browse/gtk+/tree/gtk/a11y/gtktreeviewaccessible.
> c?h=gtk-3-22#n104
> 
> should call gtk_container_cell_accessible_remove_child and 
> g_object_unref as many times as gtk_container_cell_accessible_add_child
> and create_cell_accessible_for_render were called in 
> create_cell_accessible:
> 
> https://git.gnome.org/browse/gtk+/tree/gtk/a11y/gtktreeviewaccessible.
> c?h=gtk-3-22#n404

Well, g_object_unref(cell) should be called just after gtk_container_cell_accessible_add_child. 

Anyway there is the leak
Comment 3 Massimo 2016-11-10 14:14:56 UTC
Another memory leak hit in the program above is 
here:

https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n4512

priv->hindicator is reset twice whereas priv->vindicator
never (likely a copy/paste error).