GNOME Bugzilla – Bug 108233
GdK::gc:get_screen() will return corrupt reference
Last modified: 2004-12-22 21:47:04 UTC
The following program: ------------------- snip ------------------------------ #include <gtkmm/main.h> #include <gtkmm/window.h> #include <stdio.h> int main(int argc, char *argv[]) { Gtk::Main main_runner(argc, argv); Gtk::Window foo; foo.show_all(); Glib::RefPtr<Gdk::GC> gc = Gdk::GC::create(foo.get_window()); Glib::RefPtr<const Gdk::Screen> pScreen = gc->get_screen(); //pScreen->reference(); return(0); } ------------------------- snip ---------------------- produces the following output on my machine: (bug:26672): GLib-GObject-CRITICAL **: file gobject.c: line 1337 (g_object_unref): assertion `G_IS_OBJECT (object)' failed Segmentation fault I haven't tested it on any other platform yet. I discovered this problem with gtkmm-2.2.0 (release date 01/07/2003), using gtk-2.2.1 (02/02/2003). The commented line (pScreen->reference()) fixes this problem. It seems to stem from the get_screen() methods in Gdk::GC. (source file: gdk/gdkmm/gc.cc in the gtkmm-2.2.0 source package). The following patch seems to fix this, but I haven't tested if it causes any trouble with other components: -------- snip ------- Glib::RefPtr<Screen> GC::get_screen() { // return Glib::wrap(gdk_gc_get_screen(gobj())); return Glib::wrap(gdk_gc_get_screen(gobj()), true); } Glib::RefPtr<const Screen> GC::get_screen() const { //return Glib::wrap(gdk_gc_get_screen(const_cast<GdkGC*>(gobj()))); return Glib::wrap(gdk_gc_get_screen(const_cast<GdkGC*>(gobj())), true); } -------- snip -------- The old code is still here as comments. The true forces wrap to increase the reference counter. This prevents an attempted double-free on the Gdk::Screen object returned by these functions. Hope this makes any sense. Bye, Sven
Corrected patch attached. (With help from sfs.)
Created attachment 14976 [details] [review] Fix.
Looks great. This should be applied, with a ChangeLog entry.
Committed. Murray: It seems that there are some more instances of this problem, especially in methods that were new in 2.2. Sven (sfs) is investigating this. If you don't mind I will check in such fixes with an appropriate ChangeLog entry.
Created attachment 15012 [details] [review] proposed patch.
I've discovered similar bugs in various classes. A proposed patch is attached. Unfortunately, I've got autoconf-related problems compiling the cvs branch on my system, so I could not try this out properly. Hope it works. Bye, Sven
It looks good. Almost every get_ function should use refreturn. Hopefully you have checked that these things, particularly the get_default_* ones really do expect the caller to unref. However, - "sfs 030313" doesn't mean much to me and will mean less to someone else. - Dude, you've gotta patch the ChangeLog. Sebastian, I would like to see the patches before I give permission to check them in.
I cross-checked with the gtkmm documentation (or gtk- documentation in some cases) whereever I could. I do not understand what you mean by 'they expect the caller to unref', I was under the impressions that a Glib::RefPtr unref's automagically on destruction *puzzled look*. The patch is already attached as attachement #15012 so you can check this out if you like. I'll fix the ChangeLog.
Created attachment 15021 [details] [review] ChangeLog patch
I meant the GTK+ documentation. It's GTK+ that we are wrapping.
In general, when I ask for improvements to a patch, you should revise the patch instead of adding an extra patch. Just for future reference. Then I don't have to review/apply 3 instead of just 1. By the way, the first "Fix" patch was for the gtkmm module, which is gtkmm 2.4. But it applies anyway. All 3 patches were applied.