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 437281 - gtk_button_set_image destroyes the old image
gtk_button_set_image destroyes the old image
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.11.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 451387 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-05-09 20:41 UTC by Jochen Baier
Modified: 2007-07-03 17:12 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Jochen Baier 2007-05-09 20:41:41 UTC
Please describe the problem:
using "gtk_button_set_image" with the same image (g_refed before) twice (or more) the second call to will destroy/unref the image.


Steps to reproduce:
testcase:

#include <gtk/gtk.h>

int
main (int argc, char *argv[])
{

  GtkWidget *window;
  GtkWidget *button;
	GtkWidget *image_play;
	GtkWidget *image_stop;

  gtk_init (&argc, &argv);   

  window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
	button=gtk_button_new();
  image_play = gtk_image_new_from_stock("gtk-media-play", GTK_ICON_SIZE_BUTTON);
  image_stop = gtk_image_new_from_stock("gtk-media-stop", GTK_ICON_SIZE_BUTTON); 

  gtk_container_add (GTK_CONTAINER(window), button);

  g_object_ref (G_OBJECT(image_play)); 
 
  gtk_button_set_image (GTK_BUTTON(button), image_play);
  gtk_button_set_image (GTK_BUTTON(button), image_stop);
  gtk_button_set_image (GTK_BUTTON(button), image_play); 

  gtk_widget_show_all (window);

  gtk_main ();
  return 0;
} 


"internal testcase" (using the some code gtkbutton.c is using):

include <gtk/gtk.h>

void construct (GtkWidget *window1, GtkWidget *play_image)

{

   GtkWidget *align;
   GtkWidget *box;

   g_object_ref (G_OBJECT(play_image));

    if (GTK_BIN (window1)->child) // align

    gtk_container_remove (GTK_CONTAINER (window1),

                          GTK_BIN (window1)->child);

   align = gtk_alignment_new (0.5, 0.5, 0.0, 0.0); 

   box = gtk_hbox_new (FALSE, 1);   

   gtk_box_pack_start (GTK_BOX (box), play_image, FALSE, FALSE, 0); 

   gtk_container_add (GTK_CONTAINER (align), box);

   gtk_container_add (GTK_CONTAINER (window1), align);

   gtk_widget_show_all (align);
   g_object_unref (play_image); 


}


int
main (int argc, char *argv[])
{
  GtkWidget *window1;
  GtkWidget *play_image;
  GtkWidget *stop_image;

  gtk_init (&argc, &argv);   

  window1 = gtk_window_new (GTK_WINDOW_TOPLEVEL);
  play_image = gtk_image_new_from_stock("gtk-media-play", GTK_ICON_SIZE_BUTTON);

  g_object_ref (G_OBJECT(play_image));     

  printf ("ref 1 %d\n", G_OBJECT (play_image)->ref_count); 
  construct (window1, play_image);
  printf ("ref 2 %d\n", G_OBJECT (play_image)->ref_count); 
  construct (window1, play_image);
  printf ("ref 3 %d\n", G_OBJECT (play_image)->ref_count);
  gtk_widget_show_all (window1);
 
  gtk_main ();
  return 0;
} 

maybe (working) solution:

#include <gtk/gtk.h>

void remove_child (GtkWidget *widget, gpointer data){
	printf ("remove child\n");
	gtk_container_remove (GTK_CONTAINER ((GtkContainer*) data),
	  widget);
}


void gtk_button_construct_child (GtkWidget *window1, GtkWidget *play_image)
{
   GtkWidget *align;
   GtkWidget *box;
 
   g_object_ref (G_OBJECT(play_image)); 
 
	 if (GTK_BIN (window1)->child) {
 
    GtkWidget *old_align= GTK_BIN (window1)->child;
		GtkWidget *old_box= GTK_BIN(old_align)->child;

  	gtk_container_foreach (GTK_CONTAINER(old_box), (GtkCallback) remove_child, old_box);
		gtk_container_remove (GTK_CONTAINER (window1),
				old_align);
		
	 }

   align = gtk_alignment_new (0.5, 0.5, 0.0, 0.0);  
	 box = gtk_hbox_new (FALSE, 1);    
   gtk_box_pack_start (GTK_BOX (box), play_image, FALSE, FALSE, 0);  
   gtk_container_add (GTK_CONTAINER (align), box); 
   gtk_container_add (GTK_CONTAINER (window1), align); 
   gtk_widget_show_all (align);
   g_object_unref (play_image);  

}

int
main (int argc, char *argv[])
{

  GtkWidget *window1;
  GtkWidget *togglebutton1;
	GtkWidget *play_image;
  GtkWidget *stop_image; 

  gtk_init (&argc, &argv);   

  window1 = gtk_window_new (GTK_WINDOW_TOPLEVEL);

  play_image = gtk_image_new_from_stock("gtk-media-play", GTK_ICON_SIZE_BUTTON);
	stop_image = gtk_image_new_from_stock("gtk-media-stop", GTK_ICON_SIZE_BUTTON);  

	g_object_ref (G_OBJECT(play_image));     
 
  gtk_button_construct_child (window1, play_image);
  gtk_button_construct_child (window1, stop_image); 
	gtk_button_construct_child (window1, play_image); 
	gtk_widget_show_all (window1);

  gtk_main ();
  return 0;
}    






           


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Matthias Clasen 2007-05-10 20:26:35 UTC
2007-05-10  Matthias Clasen <mclasen@redhat.com>

        * gtk/gtkbutton.c (gtk_button_set_image): Unparent the old
        image before overwriting priv->image.  (#437281, Jochen Baier)
Comment 2 Kristian Rietveld 2007-05-28 23:04:58 UTC
This commit broke something in gossip trunk, which makes it crash.  It seems that gtk_button_set_image() now tries to unparent the previous priv->image even when priv->image is not a valid widget anymore.  This patch seems to solve the issue:

Index: gtkbutton.c
===================================================================
--- gtkbutton.c	(revision 17955)
+++ gtkbutton.c	(working copy)
@@ -513,6 +513,7 @@ gtk_button_init (GtkButton *button)
   priv->xalign = 0.5;
   priv->yalign = 0.5;
   priv->align_set = 0;
+  priv->image = NULL;
   priv->image_is_stock = TRUE;
   priv->image_position = GTK_POS_LEFT;
 }
@@ -795,8 +796,6 @@ gtk_button_construct_child (GtkButton *b
       gtk_container_add (GTK_CONTAINER (align), box);
       gtk_widget_show_all (align);
 
-      g_object_unref (image);
-
       return;
     }
   
Comment 3 Richard Hult 2007-05-29 09:43:52 UTC
(This happens for other apps too, like ardour)
Comment 4 Matthias Clasen 2007-05-30 05:02:23 UTC
Hmm, initializing image to NULL is certainly ok (but should not be necessary),
but removing the unref is not. It is necessary to match the ref that is happening 
in lines 733 or 749.
Comment 5 Kristian Rietveld 2007-05-30 19:15:35 UTC
It was a kind of wild guess; I will look into it again.  (it really smells like it's something with the ref counting though).

Comment 6 Richard Hult 2007-05-30 19:18:10 UTC
I tried to write a small test case but I couldn't reproduce it :/ It still happens in gossip and ardour though. 
Comment 7 Kristian Rietveld 2007-05-30 23:23:32 UTC
As far as I can see now it appears to be an issue of the application in conjunction with libglade.  We'll try to further investigate.  When we know the exact cause maybe we can make GTK+ a bit more solid in this area instead of crashing (if it makes sense).

Comment 8 Matthias Clasen 2007-05-31 00:28:09 UTC
I'm all for that. 
Comment 9 Sebastien Bacher 2007-06-25 16:55:31 UTC
https://bugs.launchpad.net/ubuntu/+source/gtk+2.0/+bug/122141 has a simple example
Comment 10 Sebastien Bacher 2007-07-02 19:10:25 UTC
*** Bug 451387 has been marked as a duplicate of this bug. ***
Comment 11 Matthias Clasen 2007-07-03 17:12:00 UTC
2007-07-03  Matthias Clasen  <mclasen@redhat.com>

        * gtk/gtkbutton.c (gtk_button_construct_child): Don't leave
        priv->image dangling when use-stock is set to FALSE.  (#437281,
        Jochen Baier)