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 607061 - GtkPlug socket window is sometimes incorrectly unref'd (leading to crashes)
GtkPlug socket window is sometimes incorrectly unref'd (leading to crashes)
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.18.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-01-15 09:52 UTC by Karl Tomlinson
Modified: 2010-02-09 01:31 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
possible patch (1.47 KB, patch)
2010-01-18 05:30 UTC, Matthias Clasen 		none Details | Review
only add a reference to windows from gdk_window_lookup_for_display (2.65 KB, patch)
2010-01-20 10:17 UTC, Karl Tomlinson 		committed Details | Review

Description Karl Tomlinson 2010-01-15 09:52:25 UTC 
GtkPlug socket window is sometimes incorrectly unref'd (leading to crashes)

GtkPlug::socket_window is
sometimes initialized from gdk_window_lookup_for_display (display, socket_id),
   and sometimes from gdk_window_foreign_new_for_display (display, socket_id).

http://git.gnome.org/browse/gtk+/tree/gtk/gtkplug.c#n469

For gdk_window_foreign_new_for_display the reference should be released, and
is done so in gtk_plug_unrealize.

However, for gdk_window_lookup_for_display (display, socket_id) it looks like
the reference count should be incremented while held by the GtkPlug.

If that is done even for in-process embedding, then
_gtk_plug_remove_from_socket should also decrement the reference count.
An adjustment would also be necessary for the call from _gtk_socket_add_window
to _gtk_plug_add_to_socket.

Client bug report is https://bugzilla.mozilla.org/show_bug.cgi?id=539897
Comment 1 Matthias Clasen 2010-01-18 05:30:09 UTC 
Created attachment 151651 [details] [review]
possible patch

Does this patch look right to you ?
And more importantly, does it fix your problem ?
Comment 2 Karl Tomlinson 2010-01-20 10:14:30 UTC 
Review of attachment 151651 [details] [review]:

Most of this makes sense to me, thanks Matthias.
However, I don't think the extra reference should be added in the gdk_window_foreign_new_for_display case - that case seems to be working fine already.
Comment 3 Karl Tomlinson 2010-01-20 10:17:04 UTC 
Created attachment 151822 [details] [review]
only add a reference to windows from gdk_window_lookup_for_display

(Includes some of Matthias's patch.)
Comment 4 Matthias Clasen 2010-01-20 13:55:32 UTC 
An even easier fix might be to just always call gdk_window_foreign_new, since it returns a new reference regardless of whether the window already existed or not.
Comment 5 Karl Tomlinson 2010-02-09 01:31:39 UTC 
Thanks for the commit, Matthias.
Sorry I got distracted and didn't get to looking at removing gdk_window_lookup_for_display.
It would have meant a g_object_unref(socket_window) in the "if (user_data)" case (or something similar), and I wondered whether the gdk_window_lookup_for_display made for easier-to-understand code anyway.