It would be nice if the dbus_g_proxy_disconnect_signal() calls in xfconf_channel_finalize() are done in xfconf_channel_dispose(). This allows the following usage of XfconfChannel (which I'd like to use to centralize the xfconf code in the panel): if (xfconf_init (NULL)) { channel = xfconf_channel_new ("foo"); g_object_weak_ref (G_OBJECT (channel), (GWeakNotify) xfconf_shutdown, NULL); } Currently this spawns errors because the weak refs are executed between dispose and finalize and so _xfconf_get_dbus_g_proxy() will complain. Duno if you can count those signals as "drop all references to other objects" which is what dispose should do. Also: "It may be run multiple times (due to reference loops)" is not really the case here, but if it would occur, dbus-glib will only spawn a warning [1]. Anyway, any thoughts about this? I have a patch and it runs fine here (will attach it later today). [-] http://library.gnome.org/devel/gobject/unstable/gobject-The-Base-Object-Type.html#GObjectClass [1] http://www.google.com/codesearch/p?hl=en&sa=N&cd=1&ct=rc#rWC-u-FD0gU/dbus-glib-0.72/dbus/dbus-gproxy.c&q=dbus_g_proxy_disconnect_signal
Created attachment 2412 Disconnect dbus signals during dispose
Sure, looks fine. I added a bool "disposed" member to the instance struct to be sure dispose doesn't get run twice. Can't hurt, anyway... Mem usage is the same since I added the bool to a bitfield.
Er, mark fixed...
The parent class call should not be in the boolean dispose check (and you could set channel->cache to null and check for that). Anyway, since the cache entered master there is more going on here. You also need to release the cache's dbus proxy in dispose and when using something like this for bindings: xfconf_init(NULL); channel = xfconf_get_channel(...) g_object_weak_ref (G_OBJECT (channel), (GWeakNotify) xfconf_shutdown, NULL); /* ... connect some bindings */ g_object_weak_ref (G_OBJECT (object), (GWeakNotify) g_object_unref, channel); The bindings also cause problems and segfault. Anyway, I'm trying to fix all this and will attach a new patch once it's all fixed.
(In reply to comment #4) > The parent class call should not be in the boolean dispose check You sure about that...? > (and you could set channel->cache to null and check for that). Eh, doesn't matter... > Anyway, since the cache entered master there is more going on here. You also > need to release the cache's dbus proxy in dispose The cache doesn't own a proxy. > and when using something like this for bindings: > > xfconf_init(NULL); > channel = xfconf_get_channel(...) > g_object_weak_ref (G_OBJECT (channel), (GWeakNotify) xfconf_shutdown, NULL); > > /* ... connect some bindings */ > g_object_weak_ref (G_OBJECT (object), (GWeakNotify) g_object_unref, channel); Not sure what you mean here. > The bindings also cause problems and segfault. Anyway, I'm trying to fix all > this and will attach a new patch once it's all fixed. Backtrace?
(In reply to comment #5) > (In reply to comment #4) > > The parent class call should not be in the boolean dispose check > > You sure about that...? Yep, gobject is not (possibly) running it multiple times so you can block it ;-) > > Anyway, since the cache entered master there is more going on here. You also > > need to release the cache's dbus proxy in dispose > > The cache doesn't own a proxy. Maybe now you release the cache early it works fine, haven't tried it yet, but there are signals connected to the proxy. If you really want to fix this, try the devel panel and uncomment the lines in common/panel-xfconf.c. That will sprew all the warnings and crashes ^_^. IIRC the backtraces were not very useful, will try to get you some later today. If you think what i do in the panel is wrong, I'd also like to hear that of course.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > > Anyway, since the cache entered master there is more going on here. You also > > > need to release the cache's dbus proxy in dispose > > > > The cache doesn't own a proxy. > > Maybe now you release the cache early it works fine, haven't tried it yet, but > there are signals connected to the proxy. What signals? What proxy? The only dbus signals I see are connected in XfconfCache::init(), and are disconnected in XfconfCache::finalize(). Since XfconfChannel drops its reference on cache in XfconfChannel::dispose(), all dbus signals XfconfCache is connected to should be disconnected by the time XfconfChannel::dispose() is finished. The only thing I can think of is something to do with the singleton channels created with xfconf_channel_get(), but still I don't see how that would cause a problem since xfconf_shutdown() will destroy those channels before shutting down. It also 'breaks' all bindings as well (before destroying the channel singletons), so that kills any (GObject) signals connected by those. > If you really want to fix this, try the devel panel and uncomment the lines in > common/panel-xfconf.c. That will sprew all the warnings and crashes ^_^. IIRC > the backtraces were not very useful, will try to get you some later today. If > you think what i do in the panel is wrong, I'd also like to hear that of > course. I don't really have time to play with the new panel right now. Backtraces with G_DEBUG=fatal_warnings would definitely be useful.
Created attachment 2532 Increase ref count when returning a channel singleton Part of the problem was that all the singleton channels had a refcount of 1. Attached patch fixes this.
Comment on attachment 2532 Increase ref count when returning a channel singleton Err sorry, I should've read the api doc of xfconf_channel_get().
Well most of this is fixed in the panel now. Bindings still give some trouble but I've simplified the binding code in xfconf. With those changes applied the panel works flawless, so that's good news. I'll open a new bug when the changes I made in xfconf have been fully tested. Probably with a test suite attached.