connection_setup_free assumes that (io|timeout)_handler_destroy_source call the GDestroyNotify set during g_source_set_callback. But if anything else still holds a reference to the source, this doesn't happen (for example when destroying the connection in the "Disconnected" callback.
Created attachment 2301 [details] test program triggering the bug, patch is against CVS from 2005-04-04
Your attachment title sounds like you have a patch, but you didn't attach one? Or does this still need a patch? Thanks
Created attachment 2336 [details] Another test case I can reproduce this too; I only found this bug after writing it. The bug in this test case can be triggered by the bus going away. It seems that the refcounting on the underlying GSource is messed up; with debugging I found the refcount is 3 when supposed to be 1 so libdbus sits and spins nicely in the beginning of glib/dbus-gmain.c:connection_setup_free() while (cs->ios) io_handler_destroy_source (cs->ios->data); since the io_handler_source_destroyed callback is never invoked. Now to write a patch.
Another approach would be to free the io and timeout sources on disconnect, rather than connection finalize; the message queue source needs to remain post-disconnect in case there's still stuff in the queue though. In fact I'm not sure why the io and timeout sources still exist after disconnect, I'd expect the DBusWatch/DBusTimeout to be disabled at disconnection.
Created attachment 2339 [details] [review] Proposed patch OK, so how about this patch that does what is proposed in comment 4? It works for me in both the test cases and in HAL where I want to dbus_connection_unref() when the bus goes away.
Comment on attachment 2339 [details] [review] Proposed patch Patch in comment 5 isn't good because it seems there is no way to remove the existing callbacks and we're leaking the handler.
Created attachment 2340 [details] [review] Alternate patch Here is another try where we remove the element immediately thus allowing us to remove all sources. This works on the testcase and the hal example and it is valgrind clean. How does it look?
Comment on attachment 2340 [details] [review] Alternate patch I think it's important to rename source_destroyed to source_finalized. I would say returning the head of the list from destroy_source is kind of a hack... ;-)
Created attachment 2341 [details] [review] Cleaned up patch Here is a cleaned up version. OK to commit?
Havoc gave me the go ahead on IRC. I've committed the latest patch on HEAD.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.