[rfc] move activation to a helper process

Havoc Pennington hp at redhat.com
Thu Oct 19 07:27:07 PDT 2006


Hi,

This is sort of a half-review, but a few comments from skimming:

  - Sending all those DBUS_MAXIMUM_NAME_LENGTH blocks that are usually
    mostly-nul seems a little wasteful, despite premature optimization
    and all that ...
  - Why not send the .service file name instead of the bus name to
    activate, then we don't need the inefficient
    _scan_service_dir_for_desktop_file
  - the fork-without-exec is inherently somewhat confusing, I think
    a nice way to make it clearer would be to have a single
    file where all code runs in the child, exporting one
    function like bus_activation_helper_start_child(),
    where this function is called in the child immediately post-fork.
    but I don't know if this is easily done. I think it'd be
    nicer to split the BusActivationHelper struct up instead of having
    the in_helper flag, also.
  - breaking up the handle_watch function would improve readability
  - in handle_bus_pipe the "should never happen" should use _dbus_warn
    or _dbus_assert_not_reached so we can see it if it does
  - the system headers (especially unix-specific ones) aren't allowed
    in bus/*.c in general; the current exception is dir-watch-* which
    is configure-protected. string.h is allowed but almost always
    DBusString should be used instead (trivial strcmp, strlen is OK,
    but if you're doing pointer math, DBusString is much safer)
    The minimum fix is just wrap all the system calls in a trivial way
    in dbus-sysdeps-unix.[hc], better though is to come up with a
    higher-level wrapper that could work on windows ... probably on
    windows it needs to use threads instead of fork ... remember that
    the sysdeps abstractions don't need to be glib/nspr-worthy, they
    can be a little ad hoc or special purpose.
    One thought is to encapsulate the two pipes and make them look
    like a full duplex pipe (could even use socketpair when available)
    and allow using some other mechanism on windows; and then also
    encapsulate threads vs. process. so basically have "run this
    code async in another thread or process, with a full duplex
    stream to talk to it" as the abstraction. The stream might
    have its own read/write methods, since I don't think using sockets
    on windows will make sense, which means the current _dbus_read_socket
    etc. won't work.
  - officially at the moment dbus_uint64_t is "optional" (requires
    DBUS_HAVE_INT64 checks) but I think GLib and Qt both require 64-bit
    int now, so making it optional is probably stupid. I was just copying
    older glib here and I don't think anyone is aware of a
    currently-interesting platform that lacks int64.

Havoc


More information about the dbus mailing list