gdk_display_close problems ...

[Michael Meeks <michael ximian com>]

Hi there,

	So - I've been debugging an embarassing set of problems with
gdk_display_close. I'm assuming that in gtk+ for some reason we let a
number of GdkDisplay refs float around the place so that the display is
never finalized, and we only dispose it once (with the 'closed' guard).

	However - quite a simple program will provoke a SEGV eg.

#include <gtk/gtk.h>

int
main (int argc, char **argv)
{
	GdkDisplay *display;
	gtk_init (&argc, &argv);
	display = gdk_display_open (":0.0");
	gdk_display_close (display);
	return 0;	
} 

	Which approximate what OO.o is doing, although in fact we create a
GtkWindow and tear it down as well; my attached patch fixes a number of
obvious problems, however I'm slightly stumped wrt. GdkScreenX11.
gdk_screen_init calls _gdk_windowing_window_init (screen); which sets up
root_window.

	The gdk_screen_dispose NULL'd the pointer that then gdk_screen_finalize
tried to unref - causing noise. Initially I binned the NULLing of it in
gdk_screen_dispose and got:

(soffice.bin:20615): Gdk-WARNING **: losing last reference to
undestroyed window
Gdk-ERROR **: attempted to destroy root window

	What was intended here ?

	And/or am I doing something stupid / was this code tested ?

	Regards,

		Michael.

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gtk+/ChangeLog,v
retrieving revision 1.4720
diff -u -p -u -r1.4720 ChangeLog
--- ChangeLog	16 Dec 2003 00:56:42 -0000	1.4720
+++ ChangeLog	16 Dec 2003 07:03:43 -0000
@@ -1,3 +1,17 @@
+2003-12-16  Michael Meeks  <michael ximian com>
+
+	* gdk/x11/gdkevents-x11.c
+	(_gdk_x11_events_uninit_screen): double free fix.
+
+	* gdk/x11/gdkscreen-x11.c (gdk_screen_x11_finalize):
+	remove unref of root_window NULL'd in dispose.
+	(gdk_screen_x11_dispose): kill double colormap unref
+
+	* gdk/x11/gdkdisplay-x11.c
+	(gdk_display_x11_finalize): move XCloseDisplay to the
+	finalize - after the last use of xdisplay; where it
+	will only happen once cf. gdk_display_close.
+
 Tue Dec 16 01:46:46 2003  Matthias Clasen  <maclas gmx de>
 
 	Do not interpret distant clicks as double-clicks  (#116541,
Index: gdk/x11/gdkdisplay-x11.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/x11/gdkdisplay-x11.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 gdkdisplay-x11.c
--- gdk/x11/gdkdisplay-x11.c	9 Dec 2003 23:12:53 -0000	1.39
+++ gdk/x11/gdkdisplay-x11.c	16 Dec 2003 07:03:44 -0000
@@ -646,14 +646,13 @@ gdk_display_x11_dispose (GObject *object
   gint i;
   
   display_x11 = GDK_DISPLAY_X11 (object);
-  
-  for (i = 0; i < ScreenCount (display_x11->xdisplay); i++)
-    _gdk_screen_close (display_x11->screens[i]);
 
-  g_source_destroy (display_x11->event_source);
+  for (i = 0; i < ScreenCount (display_x11->xdisplay); i++)
+      _gdk_screen_close (display_x11->screens[i]);
 
-  XCloseDisplay (display_x11->xdisplay);
-  display_x11->xdisplay = NULL;
+  if (display_x11->event_source)
+    g_source_destroy (display_x11->event_source);
+  display_x11->event_source = NULL;
 
   G_OBJECT_CLASS (parent_class)->dispose (object);
 }
@@ -701,6 +700,8 @@ gdk_display_x11_finalize (GObject *objec
     g_object_unref (display_x11->screens[i]);
   g_free (display_x11->screens);
   g_free (display_x11->startup_notification_id);
+
+  XCloseDisplay (display_x11->xdisplay);
   
   G_OBJECT_CLASS (parent_class)->finalize (object);
 }
Index: gdk/x11/gdkevents-x11.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/x11/gdkevents-x11.c,v
retrieving revision 1.121
diff -u -p -u -r1.121 gdkevents-x11.c
--- gdk/x11/gdkevents-x11.c	16 Dec 2003 00:56:48 -0000	1.121
+++ gdk/x11/gdkevents-x11.c	16 Dec 2003 07:03:44 -0000
@@ -166,7 +166,8 @@ _gdk_x11_events_uninit_screen (GdkScreen
 {
   GdkScreenX11 *screen_x11 = GDK_SCREEN_X11 (screen);
 
-  xsettings_client_destroy (screen_x11->xsettings_client);
+  if (screen_x11->xsettings_client)
+    xsettings_client_destroy (screen_x11->xsettings_client);
   screen_x11->xsettings_client = NULL;
 }
 
Index: gdk/x11/gdkscreen-x11.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/x11/gdkscreen-x11.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 gdkscreen-x11.c
--- gdk/x11/gdkscreen-x11.c	21 Nov 2003 01:02:45 -0000	1.33
+++ gdk/x11/gdkscreen-x11.c	16 Dec 2003 07:03:44 -0000
@@ -284,7 +284,8 @@ gdk_screen_x11_dispose (GObject *object)
 
   _gdk_x11_events_uninit_screen (GDK_SCREEN (object));
 
-  g_object_unref (screen_x11->default_colormap);
+  if (screen_x11->default_colormap)
+    g_object_unref (screen_x11->default_colormap);
   screen_x11->default_colormap = NULL;
   
   screen_x11->root_window = NULL;
@@ -303,7 +304,7 @@ gdk_screen_x11_finalize (GObject *object
 {
   GdkScreenX11 *screen_x11 = GDK_SCREEN_X11 (object);
   /* int i; */
-  g_object_unref (screen_x11->root_window);
+  /*  g_object_unref (screen_x11->root_window); */
   
   /* Visual Part (Need to implement finalize for Visuals for a clean
    * finalize) */


-- 
 michael ximian com  <><, Pseudo Engineer, itinerant idiot