Bug 2889

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.