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 117579 - _gdk_xid_table_insert trouble
_gdk_xid_table_insert trouble
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other All
: High critical
: ---
Assigned To: gtk-bugs
gtk-bugs
: 120077 120102 123846 128424 128951 131476 131944 131963 132254 132722 132877 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-07-16 14:00 UTC by Morten Welinder
Modified: 2011-02-04 16:16 UTC
See Also:
GNOME target: 2.6.next
GNOME version: ---


Attachments
Possible patch (9.57 KB, patch)
2003-12-14 22:12 UTC, Soren Sandmann Pedersen
none Details | Review

Description Morten Welinder 2003-07-16 14:00:15 UTC
Starting Gnumeric (bonobo version) yields the problem below.  It looks
like _gdk_xid_table_insert and gdk_window_reparent conflict.

(bonobo and gtk+ out of cvs HEAD a few days ago.)


      FMR: Free memory read (2 times)
      This is occurring while in:
            gdk_xid_equal  [gdkxid.c:123 pc=0xfabb1814]
            _gdk_xid_table_insert [gdkxid.c:52 pc=0xfabb1430]
            gdk_window_new [gdkwindow-x11.c:620 pc=0xfab9e1b8]
            gtk_plug_realize [gtkplug.c:538 pc=0xfafd0f98]
            bonobo_plug_realize [bonobo-plug.c:161 pc=0xfb343868]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
            g_signal_emit_valist [gsignal.c:2103 pc=0xfa4bed60]
            g_signal_emit  [gsignal.c:2147 pc=0xfa4bf214]
            gtk_widget_realize [gtkwidget.c:2020 pc=0xfafa3540]
            gtk_window_show [gtkwindow.c:3521 pc=0xfafc44ec]
            gtk_plug_show  [gtkplug.c:573 pc=0xfafd1194]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
            g_signal_emit_valist [gsignal.c:2103 pc=0xfa4bed60]
            g_signal_emit  [gsignal.c:2147 pc=0xfa4bf214]
            gtk_widget_show [gtkwidget.c:1753 pc=0xfafa1f00]
            impl_Bonobo_Control_getWindowId [bonobo-control.c:369
pc=0xfb33ce9c]
            _ORBIT_skel_small_Bonobo_Control_getWindowId
[Bonobo-common.c:2172 pc=0xfa94b704]
            ORBit_c_stub_invoke [poa.c:2482 pc=0xfa746914]
            Bonobo_Control_getWindowId [Bonobo-stubs.c:2205 pc=0xfa955348]
      Reading 4 bytes from 0xc9ce20 in the heap.
      Address 0xc9ce20 is 24 bytes into a freed  block at 0xc9ce08 of 28 bytes.
      This block was allocated from:
            malloc         [rtlib.o pc=0x66cb8]
            calloc         [rtlib.o pc=0x67e3c]
            g_malloc0      [gmem.c:153 pc=0xf9fd0f18]
            _gdk_x11_window_get_toplevel [gdkwindow-x11.c:160 pc=0xfab9c310]
            gdk_window_new [gdkwindow-x11.c:652 pc=0xfab9e344]
            gtk_plug_realize [gtkplug.c:538 pc=0xfafd0f98]
            bonobo_plug_realize [bonobo-plug.c:161 pc=0xfb343868]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
            g_signal_emit_valist [gsignal.c:2103 pc=0xfa4bed60]
            g_signal_emit  [gsignal.c:2147 pc=0xfa4bf214]
            gtk_widget_realize [gtkwidget.c:2020 pc=0xfafa3540]
            gtk_window_show [gtkwindow.c:3521 pc=0xfafc44ec]
            gtk_plug_show  [gtkplug.c:573 pc=0xfafd1194]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
            g_signal_emit_valist [gsignal.c:2103 pc=0xfa4bed60]
            g_signal_emit  [gsignal.c:2147 pc=0xfa4bf214]
            gtk_widget_show [gtkwidget.c:1753 pc=0xfafa1f00]
            impl_Bonobo_Control_getWindowId [bonobo-control.c:369
pc=0xfb33ce9c]
            _ORBIT_skel_small_Bonobo_Control_getWindowId
[Bonobo-common.c:2172 pc=0xfa94b704]
      There have been 941 frees since this block was freed from:
            free           [rtlib.o pc=0x66fec]
            g_free         [gmem.c:186 pc=0xf9fd1078]
            gdk_window_reparent [gdkwindow-x11.c:1537 pc=0xfaba1ed0]
            _gtk_plug_add_to_socket [gtkplug.c:243 pc=0xfafcfd7c]
            gtk_socket_add_window [gtksocket.c:972 pc=0xfafd63d8]
            gtk_socket_add_id [gtksocket.c:309 pc=0xfafd39c4]
            bonobo_socket_add_id [bonobo-socket.c:348 pc=0xfb34a564]
            bonobo_control_frame_get_remote_window
[bonobo-control-frame.c:328 pc=0xfb338454]
            bonobo_socket_realize [bonobo-socket.c:76 pc=0xfb349228]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
            g_signal_emit_valist [gsignal.c:2103 pc=0xfa4bed60]
            g_signal_emit  [gsignal.c:2147 pc=0xfa4bf214]
            gtk_widget_realize [gtkwidget.c:2020 pc=0xfafa3540]
            gtk_widget_map [gtkwidget.c:1937 pc=0xfafa2ed8]
            gtk_container_map_child [gtkcontainer.c:2342 pc=0xface8cc4]
            gtk_bin_forall [gtkbin.c:164 pc=0xfac7c504]
            gtk_container_forall [gtkcontainer.c:1263 pc=0xface45ac]
            gtk_container_map [gtkcontainer.c:2350 pc=0xface8d4c]
            g_cclosure_marshal_VOID__VOID [gmarshal.c:77 pc=0xfa4c3fac]
            g_type_class_meta_marshal [gclosure.c:514 pc=0xfa490284]
            g_closure_invoke [gclosure.c:437 pc=0xfa48fbcc]
            signal_emit_unlocked_R [gsignal.c:2274 pc=0xfa4c1224]
Comment 1 Owen Taylor 2003-07-16 15:04:41 UTC
This is likely caused because when you turn a window
from a toplevel to a non-toplevel, the focus window
isn't removed from the XID table, but the pointer
to it is lost, so it is never removed.

But actually, the focus_window needs to be destroyed/
created at conversion time.
Comment 2 Morten Welinder 2003-07-16 17:27:02 UTC
For what it is worth, just removing the *focus* window from the hash
(as suggested on irc) is not enough.  The same problem remains.

The thing that is being freed at gdkwindow-x11.c:1537, i.e., the
toplevel is really in the hash.
Comment 3 Owen Taylor 2003-07-16 17:47:01 UTC
The focus window XID in the hash points to the toplevel window.
Comment 4 Owen Taylor 2003-08-15 14:25:21 UTC
GTK+-2.4 specific bug.
Comment 5 Owen Taylor 2003-08-17 18:07:03 UTC
*** Bug 120077 has been marked as a duplicate of this bug. ***
Comment 6 Andreas J. Guelzow 2003-09-13 19:17:50 UTC
*** Bug 120102 has been marked as a duplicate of this bug. ***
Comment 7 Soren Sandmann Pedersen 2003-12-09 16:39:34 UTC
*** Bug 123846 has been marked as a duplicate of this bug. ***
Comment 8 Soren Sandmann Pedersen 2003-12-09 16:39:42 UTC
*** Bug 128424 has been marked as a duplicate of this bug. ***
Comment 9 Soren Sandmann Pedersen 2003-12-14 09:20:43 UTC
*** Bug 128951 has been marked as a duplicate of this bug. ***
Comment 10 Soren Sandmann Pedersen 2003-12-14 22:12:38 UTC
Created attachment 22443 [details] [review]
Possible patch
Comment 11 Soren Sandmann Pedersen 2003-12-14 22:14:29 UTC
The patch I just attached is an attempt at a fix, but it needs a
thorough review, because (a) I don't know a whole lot about this area,
and (b) I haven't tested it a lot (there aren't that many applications
that reparent windows).
Comment 12 Andrew Sobala 2003-12-17 17:54:53 UTC
Migrating keywords, adding interested parties from bug 128951 to Cc.
(Please remove yourself if you're not interested :)
Comment 13 Owen Taylor 2003-12-20 18:12:57 UTC
===
+  size_hints.flags = PSize;
+  size_hints.width = impl->width;
+  size_hints.height = impl->height;
+  
+  check_leader_window_title (screen_x11->display);
+  
+  /* FIXME: Is there any point in doing this? Do any WM's pay
+   * attention to PSize, and even if they do, is this the
+   * correct value???
+   */
+  XSetWMNormalHints (xdisplay, xid, &size_hints);
===

Would be good to fix the ordering here. (Existing problem.)

====
     case GDK_WINDOW_ROOT:
     case GDK_WINDOW_FOREIGN:
       /* Now a toplevel */
-      if (GDK_WINDOW_TYPE (window) == GDK_WINDOW_CHILD)
-	set_wm_protocols (window);
+      if (impl->toplevel_window_type != -1)
+	GDK_WINDOW_TYPE (window) = impl->toplevel_window_type;
+      if (GDK_WINDOW_TYPE (window) != GDK_WINDOW_CHILD &&
+	  GDK_WINDOW_TYPE (window) != GDK_WINDOW_FOREIGN)
+	{
+	  setup_toplevel_window (window, new_parent);
+	}
       break;
===

I think you should have:

      if (impl->toplevel_window_type != -1)
	GDK_WINDOW_TYPE (window) = impl->toplevel_window_type;
      else if (GDK_WINDOW_TYPE (window) == GDK_WINDOW_CHILD)
	GDK_WINDOW_TYPE (window) = GDK_WINDOW_TOPLEVEL;

So that we leave GDK_WINDOW_CHILD windows reparented to the
root with some sort of sane type. 

Note that the old:

      if (GDK_WINDOW_TYPE (window) == GDK_WINDOW_CHILD)
	set_wm_protocols (window);

Was trying to implement the logic "if the window is *becoming*
a toplevel, then set the wm_protocols stuff" Since setting
all the toplevel properties is expensive, I think it would
be good to preserve this. What I might do is add a macro
at the top of the file:

#define WINDOW_IS_TOPLEVEL(window) \
  (GDK_WINDOW_TYPE (window) != GDK_WINDOW_CHILD && \
   GDK_WINDOW_TYPE (window) != GDK_WINDOW_TOPLEVEL)

then you can have 

 gboolean window_was_toplevel = WINDOW_IS_TOPLEVEL (window);

something like that. That could be also used in the couple
of other places where the test is done.
Comment 14 Soren Sandmann Pedersen 2003-12-21 16:37:46 UTC
Committed with the suggested changes.

Sun Dec 21 17:34:22 2003  Soeren Sandmann  <sandmann@daimi.au.dk>

	* gdk/x11/gdkwindow-x11.c (gdk_window_reparent): Set the right
	properties when the window becomes a toplevel. When a window that
	was previously a toplevel becomes a toplevel again, restore its
	window type. Also make sure the focus window is removed from the
	XID hash when it is destroyed. (#117579, reported by Morten
	Welinder, patch reviewed by Owen Taylor).
Comment 15 Matthew Gatto 2004-01-17 22:02:59 UTC
*** Bug 131476 has been marked as a duplicate of this bug. ***
Comment 16 Soren Sandmann Pedersen 2004-01-20 19:26:05 UTC
*** Bug 131963 has been marked as a duplicate of this bug. ***
Comment 17 Morten Welinder 2004-01-27 02:40:14 UTC
*** Bug 131944 has been marked as a duplicate of this bug. ***
Comment 18 Owen Taylor 2004-01-29 15:57:35 UTC
*** Bug 132722 has been marked as a duplicate of this bug. ***
Comment 19 Elijah Newren 2004-01-29 23:02:54 UTC
*** Bug 132877 has been marked as a duplicate of this bug. ***
Comment 20 Elijah Newren 2004-01-29 23:04:54 UTC
*** Bug 132254 has been marked as a duplicate of this bug. ***