Re: GDBus/GVariant plans for next GLib release

[David Zeuthen <david fubar dk>]

Hey Alex,

On Fri, 2009-11-06 at 21:05 +0100, Alexander Larsson wrote:
> On Wed, 2009-10-14 at 21:34 -0400, David Zeuthen wrote:
> 
> >  http://cgit.freedesktop.org/~david/gdbus-standalone/
> 
> I just read through this for basic review, and I must say that I like
> it. Very nice stuff. Some comments:

Thanks for reading through it!

> In nautilus I'd like to have an object path which is basically a pointer
> to another object. Say I have these objects:
> 
> /nautilus/window/1/tabs/1/view
> /nautilus/window/1/tabs/2/view
> 
> Then i want something like:
> 
> /nautilus/window/1/active_view
> 
> That always points to the view object in the active tab.
> 
> Now, i could constantly be re-creating this object all the time, but
> thats not really very nice. What i instead want to do is basically
> register a subtree at /nautilus/window/1/active_view which dynamically
> looks up which tab is active and then re-routes to the right tab.
> 
> To implement this cleanly I would need to be able to lookup an object at
> a given path (returning the ifaces, vtables and user data) and to
> enumerate the children of a given path. All this would be in-client
> registered objects/subtrees only, and is enough to implement
> GDBusSubtreeVTable for the "pointer subtree".

Yeah, I guess that's a reasonable thing to add. How about?

 gboolean
 g_dbus_connection_lookup_object (GDBusConnection *connection,
                        const gchar           *object_path,
                        const gchar           *interface_name,
                        GDBusInterfaceInfo   **out_introspection_data,
                        GDBusInterfaceVTable **out_vtable,
                        gpointer              *out_user_data,
                        guint                 *out_id);

 gboolean
 g_dbus_connection_lookup_subtree (GDBusConnection *connection,
                        const gchar           *object_path,
                        GDBusInterfaceVTable **out_vtable,
                        GDBusSubtreeFlags     *out_flags,
                        gpointer              *out_user_data,
                        guint                 *out_id);

> I don't see any call to dbus_threads_init() or
> dbus_threads_init_default(). Surely this should be done if gthread is
> enabled. Ideally in g_thread_init_glib(), but thats a bit hard for
> something in libgio...

Yeah, oops, good point - I guess we should do this in gdbusconnection.c

> I'd like it to be possible to specify a non-default main context when
> creating a connection with g_dbus_connection_new() or
> g_dbus_connection_bus_get_private().

Hmm, yeah, I was thinking about that. It's for specifying the mainloop
for processing D-Bus messages, right?

I was actually thinking that it might be nicer to always have a
dedicated thread for handling D-Bus messages. This would be shared by
all connections and it would just pop messages to the mainloops for
method invocations and signals.

This is feasible even if g_thread_init() has not be called, we'd just
use libpthread directly (which libdbus-1 pulls in already).

> g_dbus_connection_send_dbus_1_message_with_reply() has a race condition
> if called on a thread other than the one dispatching the reply. The
> pending call may be completed before dbus_pending_call_set_notify() is
> reached. See _g_dbus_connection_call_async in gvfs/common/gdbusutils.c
> for a possible workaround.

Will do.

> The way e.g. properties are implemented is very static. There is no way
> to expose an object with a dynamic set of properties (apart from
> removing and re-adding the object with new introspection data each time
> dynamic properties/methods can't be implemented? (lazy, i.e. not remove
> and re-add each time they change). I guess this is sort of weird wrt how
> dbus usually works, but might be useful when exporting an object from a
> more dynamic language like javascript or python.

Hmm, yeah, but remember that D-Bus properties (like GObject properties)
is part of the actual type - e.g. they are declared in introspection
data. So it would be very weird to have dynamic properties, it would be
similar to having dynamic properties on GObject.

If you want dynamic properties it's much better to just define your own
interfaces/methods for this (like e.g. HAL does).

> Maybe we should not claim to implement org.freedesktop.DBus.Properties
> if set/get_property are NULL?

Yeah, probably not.

> I'm not sure if I missed it, because i didn't try it out, but how do
> registered object with a a child that is specified by registering a
> subtree get enumerated when introspecting the registered object?

I'm not sure I understand this question - can you clarify?

> The foo_bar_error_quark example in gdbuserror.c clearly should just call
> a standard helper that does all this, just given the pointer to an array
> of data.

Yeah, I guess we can add some convenience for that.

> org.gtk.GDBus.UnmappedGError.Quark0xHEXENCODED_QUARK_NAME_.Code_ERROR_CODE
> Why use a hex encoding for the quark name? Most gerror quark names are
> quite readable, and it would be nice to preserve this in the encoding
> used.

Yeah, we can choose another encoding that hex. The problem is that D-Bus
is very strict about what characters it accepts for names - in
particular, '-' is not accepted. So we'd need some kind of way to escape
things. That's doable though.

> There are no bindings for dbus_connection_flush, which might be nice to
> e.g. ensure that the reply sent from a message handler is immediately
> sent since we may block or whatnot later. Maybe we want this?

Yup, good point.

> gdbusnameowning.c:request_name_cb() - Isn't it racy to not subscribe to
> NameLost/NameAquired until we're processing the request name reply? 
> We could have gotten a NameLost message inbetween the RequestName reply
> gotten and request_name_cb being handled by the mainloop, as the
> request_name_cb is called from another idle.

Hmm, yeah. I need to look at that.

> g_dbus_server_new() wants a way to specify the GMainContext to use.
> 
> gdbusprivate.c has a bunch of code with a copyright notice from dbus
> that is AFL 2.1 or GPL2+. I.e. not LGPL. This seems like a problem for
> glib integration.

Maybe we can use the code from GVfs for this?

> Are you aware of:
> #define	G_PARAM_STATIC_STRINGS (G_PARAM_STATIC_NAME |
> G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)
> It saves some typing.

Good point.

Btw, what do you think about

http://mail.gnome.org/archives/gtk-devel-list/2009-October/msg00075.html

I was going to do this as well.

Thanks,
David