Dbus-Glib has a bug where it doesn't process events to a non-default main loop context. e.g. (does not work) DBusGConnection *bus; GError *error = NULL; GMainContext *context = g_main_context_new(); GMainLoop *mainloop = g_main_loop_new (context, FALSE); bus = dbus_g_bus_get(DBUS_BUS_SESSION, &error); dbus_connection_setup_with_g_main (dbus_g_connection_get_connection(bus), context); -------------- I looked into this problem and this is what I found out (in dbus/dbus-gmain.c): When DBus-GLib switches to a new context, it calls add_watch() to attach an IOHandler to poll the file descriptor used for communication. Immediately afterward, DBus invokes DBus-GLib's remove_watch() function, which removes the new IOHandler, instead of the one corresponding to the previous context. Also, the previous IOHandler should be already freed the second time connection_setup_add_watch is called (line 277: dbus_watch_set_data (watch, handler, io_handler_watch_freed) ). I believe remove_watch() should not be called after add_watch(). ---------------- Suggested Patch: diff -urN dbus-glib-orig/dbus/dbus-gmain.c dbus-glib-patched/dbus/dbus-gmain.c --- dbus-glib-orig/dbus/dbus-gmain.c 2009-09-01 17:09:06.919358522 -0400 +++ dbus-glib-patched/dbus/dbus-gmain.c 2009-09-01 17:25:41.237368327 -0400 @@ -591,7 +591,7 @@ if (!dbus_connection_set_watch_functions (connection, add_watch, - remove_watch, + NULL, watch_toggled, cs, NULL)) goto nomem;
I believe the call to dbus_connection_set_timeout_functions has the same bug. New suggested patch: diff -urN dbus-glib-orig/dbus/dbus-gmain.c dbus-glib-patched/dbus/dbus-gmain.c --- dbus-glib-orig/dbus/dbus-gmain.c 2009-09-01 17:09:06.919358522 -0400 +++ dbus-glib-patched/dbus/dbus-gmain.c 2009-09-01 18:32:46.203038866 -0400 @@ -591,14 +591,14 @@ if (!dbus_connection_set_watch_functions (connection, add_watch, - remove_watch, + NULL, watch_toggled, cs, NULL)) goto nomem; if (!dbus_connection_set_timeout_functions (connection, add_timeout, - remove_timeout, + NULL, timeout_toggled, cs, NULL)) goto nomem;
Created attachment 29259 [details] [review] allows API user to switch to non-default main loop context
Hmm, OK without investigating in depth here, my first take would be that you really want an API that allows you to get a private connection already set up with your mainloop. What about a patch that builds on the just committed bug 19623 and adds dbus_g_bus_get_private_with_mainloop or something?
(In reply to comment #3) > Hmm, OK without investigating in depth here, my first take would be that you > really want an API that allows you to get a private connection already set up > with your mainloop. > > What about a patch that builds on the just committed bug 19623 and adds > dbus_g_bus_get_private_with_mainloop or something? Or since the just committed patch hasn't been released yet, we could change it to take a mainloop.
Created attachment 29725 [details] [review] dbus_g_bus_get_private creates private connection and attaches to specified mainloop context I modified the patch to take in a GMainContext* parameter. While we still can't change contexts after the connection is setup, this patch is a minimal change and provides a way to setup private connections with a specified thread context.
(Sorry for the slow reply) I'd like a patch that at least adds something to tests/name-test/test-dbus-glib.c.
Colin applied the equivalent of this patch in 0.84, but there still wasn't a complete test. Here's one.
Created attachment 45864 [details] [review] Add a feature test for fd.o #23633, non-default main context
Created attachment 68927 [details] [review] Add a feature test for fd.o #23633, non-default main context Original patch was good but forgot the test/core/run-test.sh bits. Updated and rebased patch attached.
Comment on attachment 68927 [details] [review] Add a feature test for fd.o #23633, non-default main context Review of attachment 68927 [details] [review]: ----------------------------------------------------------------- This is complete nitpicking but when two developers have contributed something, I prefer one of these forms: From: Bob <bob@example.com> Subject: do something Based on a patch from Alice. or From: Alice <alice@example.com> Subject: do something [fixed indentation -Bob]
Comment on attachment 68927 [details] [review] Add a feature test for fd.o #23633, non-default main context Review of attachment 68927 [details] [review]: ----------------------------------------------------------------- I'll --amend and apply this. ::: test/core/private.c @@ +1,1 @@ > +/* Feature test for freedesktop.org #23633 - non-default main context You looked at this and didn't object, so I assume my code is OK :-) ::: test/core/run-test.sh @@ +49,4 @@ > ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-variant-recursion || die "test-variant-recursion failed" > ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-gvariant || die "test-gvariant failed" > ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-30574 || die "test-30574 failed" > + ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/core/test-private || die "test-private failed" Yes, fine. Sorry I forgot this!
(In reply to comment #10) > Comment on attachment 68927 [details] [review] [review] > Add a feature test for fd.o #23633, non-default main context > > Review of attachment 68927 [details] [review] [review]: > ----------------------------------------------------------------- > > This is complete nitpicking but when two developers have contributed > something, I prefer one of these forms: > > From: Bob <bob@example.com> > Subject: do something > > Based on a patch from Alice. > > or > > From: Alice <alice@example.com> > Subject: do something > > [fixed indentation -Bob] I hadn't considered my changes significant enough to add my name to the list. I'm fine with your credit.
(In reply to comment #11) > Comment on attachment 68927 [details] [review] [review] > Add a feature test for fd.o #23633, non-default main context > > Review of attachment 68927 [details] [review] [review]: > ----------------------------------------------------------------- > > I'll --amend and apply this. > > ::: test/core/private.c > @@ +1,1 @@ > > +/* Feature test for freedesktop.org #23633 - non-default main context > > You looked at this and didn't object, so I assume my code is OK :-) Yes, that seemed fine to me.
Reworded to credit you and pushed. Fixed in git for 0.102, thanks.
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.