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 304608 - unable to close last GdkDisplay
unable to close last GdkDisplay
Status: RESOLVED DUPLICATE of bug 85715
Product: gtk+
Classification: Platform
Component: Backend: X11
2.6.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2005-05-18 05:52 UTC by Denis Vlasenko
Modified: 2005-05-18 06:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.97 KB, patch)
2005-05-18 05:54 UTC, Denis Vlasenko
none Details | Review

Description Denis Vlasenko 2005-05-18 05:52:53 UTC
Please describe the problem:
For background why do I need this, see
https://bugzilla.mozilla.org/show_bug.cgi?id=293007

I initialize gtk with this helper:

extern "C" int //gboolean
gtk_parse_args(int *argc, char ***argv);
// This one is copied from gtk source,
// modified to NOT leak GdkDisplay*:
static
GdkDisplay* //gboolean
vda_gtk_init_check(int *argc, char ***argv)
{
  if(!gtk_parse_args(argc, argv))
    return FALSE;

  return gdk_display_open_default_libgtk_only();
}

...
GdkDisplay *vda_dpy = vda_gtk_init_check(&argc, &argv);
/* the main program */
gdk_display_close(vda_dpy);
...

Program segfaults inside gdk_display_close().
This patch fixes it:

--- gdkdisplay-x11.c.orig       Wed Mar 16 05:25:09 2005
+++ gdkdisplay-x11.c    Wed May 18 08:47:00 2005
@@ -699,9 +699,11 @@ gdk_display_x11_dispose (GObject *object
 {
   GdkDisplayX11 *display_x11;
   gint i;
-
   display_x11 = GDK_DISPLAY_X11 (object);

+  if(!display_x11->xdisplay)
+    return;
+
   for (i = 0; i < ScreenCount (display_x11->xdisplay); i++)
     _gdk_screen_close (display_x11->screens[i]);

@@ -734,7 +736,8 @@ gdk_display_x11_finalize (GObject *objec
   g_hash_table_destroy (display_x11->atom_from_virtual);
   g_hash_table_destroy (display_x11->atom_to_virtual);
   /* Leader Window */
-  XDestroyWindow (display_x11->xdisplay, display_x11->leader_window);
+  if(display_x11->xdisplay)
+    XDestroyWindow (display_x11->xdisplay, display_x11->leader_window);
   /* list of filters for client messages */
   g_list_free (display_x11->client_filters);
   /* List of event window extraction functions */
@@ -752,8 +755,9 @@ gdk_display_x11_finalize (GObject *objec
     g_object_unref (tmp->data);
   g_list_free (display_x11->input_windows);
   /* Free all GdkScreens */
-  for (i = 0; i < ScreenCount (display_x11->xdisplay); i++)
-    g_object_unref (display_x11->screens[i]);
+  if(display_x11->xdisplay)
+    for (i = 0; i < ScreenCount (display_x11->xdisplay); i++)
+      g_object_unref (display_x11->screens[i]);
   g_free (display_x11->screens);
   g_free (display_x11->startup_notification_id);

@@ -829,14 +833,16 @@ gdk_x11_display_get_xdisplay (GdkDisplay
 void
 _gdk_windowing_set_default_display (GdkDisplay *display)
 {
-  GdkDisplayX11 *display_x11 = GDK_DISPLAY_X11 (display);
+  GdkDisplayX11 *display_x11;
   const gchar *startup_id;

-  if (display)
-    gdk_display = GDK_DISPLAY_XDISPLAY (display);
-  else
+  if (!display) {
     gdk_display = NULL;
+    return;
+  }

+  gdk_display = GDK_DISPLAY_XDISPLAY (display);
+  display_x11 = GDK_DISPLAY_X11 (display);
   g_free (display_x11->startup_notification_id);
   display_x11->startup_notification_id = NULL;

Gory details:

valgrind says:
Invalid read of size 4
at 0x345AA068: _gdk_windowing_set_default_display (x11/gdkdisplay-x11.c:788)
by 0x3459516D: gdk_display_manager_set_default_display (gdkdisplaymanager.c:257)
by 0x3458EE54: gdk_display_dispose (../../gdk/gdkdisplay.c:177)
by 0x345A9DEB: gdk_display_x11_dispose (../../../gdk/x11/gdkdisplay-x11.c:661)
by 0x3465CC34: g_object_run_dispose (../../gobject/gobject.c:602)
by 0x3458EEE5: gdk_display_close (../../gdk/gdkdisplay.c:207)
by 0x804C135: main (in /.share/usr/app/mozilla-2005-05-08/mozilla-bin)
Address 0xE8 is not stack'd, malloc'd or (recently) free'd

What happens:
gdk_display_x11_dispose (GObject *object)
{
  GdkDisplayX11 *display_x11;
  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);

  XCloseDisplay (display_x11->xdisplay);
  display_x11->xdisplay = NULL;

  G_OBJECT_CLASS (parent_class)->dispose (object);  <-- ***
}


gdk_display_dispose (GObject *object)
{
  GdkDisplay *display = GDK_DISPLAY_OBJECT (object);

  g_list_foreach (display->queued_events, (GFunc)gdk_event_free, NULL);
  g_list_free (display->queued_events);
  display->queued_events = NULL;
  display->queued_tail = NULL;

  _gdk_displays = g_slist_remove (_gdk_displays, object);

  if (gdk_display_get_default() == display)
    gdk_display_manager_set_default_display (gdk_display_manager_get(), NULL); 
 <-- ***
}


void
gdk_display_manager_set_default_display (GdkDisplayManager *display_manager,
                                         GdkDisplay        *display)
{
  default_display = display;

  _gdk_windowing_set_default_display (display);  <-- *** display == NULL

  g_object_notify (G_OBJECT (display_manager), "default_display");
}


_gdk_windowing_set_default_display (GdkDisplay *display)
{
  GdkDisplayX11 *display_x11 = GDK_DISPLAY_X11 (display); <-- *** NULL deref
  const gchar *startup_id;

  if (display)
    gdk_display = GDK_DISPLAY_XDISPLAY (display);
  else
    gdk_display = NULL;

  g_free (display_x11->startup_notification_id);
  display_x11->startup_notification_id = NULL;

  startup_id = g_getenv ("DESKTOP_STARTUP_ID");
  if (startup_id && *startup_id != '\0')
    {
      if (!g_utf8_validate (startup_id, -1, NULL))
        g_warning ("DESKTOP_STARTUP_ID contains invalid UTF-8");
      else
        display_x11->startup_notification_id = g_strdup (startup_id);

      /* Clear the environment variable so it won't be inherited by
       * child processes and confuse things.  unsetenv isn't portable,
       * right...
       */
      putenv ("DESKTOP_STARTUP_ID=");

      /* Set the startup id on the leader window so it
       * applies to all windows we create on this display
       */
      XChangeProperty (display_x11->xdisplay,
                       display_x11->leader_window,
                       gdk_x11_get_xatom_by_name_for_display (display,
"_NET_STARTUP_ID"),
                       gdk_x11_get_xatom_by_name_for_display (display,
"UTF8_STRING"), 8,
                       PropModeReplace,
                       startup_id, strlen (startup_id));
    }
}

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Shall gtk_init leak GdkDisplay like this? can it simply return GdkDisplay*?
Comment 1 Denis Vlasenko 2005-05-18 05:54:44 UTC
Created attachment 46583 [details] [review]
Patch
Comment 2 Matthias Clasen 2005-05-18 06:17:11 UTC

*** This bug has been marked as a duplicate of 85715 ***