Simon McVittie [Fri, 25 Feb 2011 17:46:54 +0000 (17:46 +0000)]
Don't finalize sent or dispatched messages while under the connection lock
Finalizing a message can trigger callbacks; that's bad, if we have a
connection locked.
In particular, if a message is received by the "left side", passed to
the "right side" and sent (as in test/relay.c (see the diagram there)
or in dbus-daemon), then finalizing that message could result in the
live messages counter for the left side, and the outgoing messages counter
for the right side, both being decremented while under either side's
lock.
After a message is dispatched on the left side, finalizing it now drops
the lock temporarily, to avoid this problem.
After a message is sent on the right side, finalizing it is now deferred
until the right side unlocks, by moving it to a new queue of
"expired messages" which is automatically cleared every time we release
the lock.
The "live messages" counter for the "left" connection will now explicitly
take the left connection's lock before decrementing, to avoid
manipulating watches without a lock.
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34393
Simon McVittie [Fri, 25 Feb 2011 17:21:44 +0000 (17:21 +0000)]
When attaching counters to messages, don't automatically notify callbacks
In all the places where counters are added, we're under a lock. The caller
knows what effect adding the counter might have, and can replicate it
in a lock-safe way if necessary.
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34393
Simon McVittie [Fri, 25 Feb 2011 17:08:59 +0000 (17:08 +0000)]
Comment some places where it's OK to unref a message despite holding locks
In general, dbus_message_unref should be avoided while holding locks,
because it can invoke arbitrary user callbacks (via attached data, or
via DBusCounter).
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34393
Simon McVittie [Tue, 14 Jun 2011 09:10:24 +0000 (10:10 +0100)]
man pages: replace all unescaped hyphen/minus characters with \-
In a man page, "-" officially means a typographical (Unicode) hyphen,
which frequently breaks the ability to copy and paste code examples from
a man page. "\-" means the ASCII hyphen/minus character. See
<http://lintian.debian.org/tags/hyphen-used-as-minus-sign.html> for
more details.
Rather than trying to distinguish between hyphens, em-dashes and
hyphen/minus, I just replaced all ambiguous hyphens with \- by applying
this vim command repeatedly until it didn't find anything:
%s/\(^\|[^\\]\)-/\1\\-/g
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38284 Reviewed-by: Lennart Poettering <lennart@poettering.net>
Simon McVittie [Wed, 15 Jun 2011 11:43:15 +0000 (12:43 +0100)]
Use EXEEXT when running tests from another directory, and skip bus-test-launch-helper on non-Unix
This is necessary when cross-compiling from Linux to mingw32 and running
the resulting tests under Wine. (This partially works! Some tests fail,
though.)
Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Simon McVittie [Mon, 6 Jun 2011 15:12:05 +0000 (16:12 +0100)]
Build docs after running tests, and remove redundant DIST_SUBDIRS
If DIST_SUBDIRS isn't set, it defaults to SUBDIRS, so it's just noise.
Running tests before building documentation is an easy way to speed up the
hack/make check/fix cycle, by not wasting time rebuilding the
documentation (which is often slow) until all the tests compile and pass.
https://bugs.freedesktop.org/show_bug.cgi?id=34405 Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Simon McVittie [Wed, 22 Jun 2011 10:59:32 +0000 (11:59 +0100)]
_dbus_connect_tcp_socket_with_nonce: don't create an extra fd (which is then leaked)
This block should have been deleted in 2007, when IPv6 support was added:
previously, the fd allocated at the beginning of the function was used
for connect(), but for IPv6 support, the socket() call has to be inside
the loop over getaddrinfo() results so its address family can be changed.
Do not allow eavedropping unless rule owner explicitely declare it
Adds "eavesdrop=true" as a match rule, meaning that the owner
intend to eavedrop.
Otherwise the owner will receive only broadcasted messages and the ones
meant to be delivered to it.
[plus a typo fix in an error message -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=37890
Bug-NB: NB#269748 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Introduce static generic _dbus_connection_register_object_path()
function to remove duplicate code for registration object paths.
Having *four* almost the same functions is error-prone and hard
to follow as well.
Signed-off-by: Jiří Klimeš <jklimes@redhat.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38874
Jiří Klimeš [Fri, 1 Jul 2011 10:58:11 +0000 (12:58 +0200)]
DBusConnection: use DBUS_ERROR_OBJECT_PATH_IN_USE instead of DBUS_ERROR_ADDRESS_IN_USE
While registering an object path the possible error is
DBUS_ERROR_OBJECT_PATH_IN_USE, not DBUS_ERROR_ADDRESS_IN_USE (the error
is set by _dbus_object_tree_register()).
Signed-off-by: Jiří Klimeš <jklimes@redhat.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38874
Simon McVittie [Wed, 22 Jun 2011 13:57:56 +0000 (14:57 +0100)]
Remove maximum length field from DBusString
The source code says it's "a historical artifact from a feature that
turned out to be dumb". Respond accordingly!
This reduces sizeof (DBusString) by 20% on ILP32 architectures, which
can't hurt. (No reduction on LP64 architectures that align pointers
naturally, unfortunately.)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38570 Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Simon McVittie [Thu, 7 Apr 2011 16:36:54 +0000 (17:36 +0100)]
DBusLoop: store watches in a hash table keyed by fd
This means we don't need to build up the watches_for_fds array, because
it's quick to find the watches based on the fd.
This also fixes fd.o #23194, because we only put each fd in the
array of DBusPollFD once, with the union of all its watches' flags; we
can no longer get more entries in the array than our number of file
descriptors, which is capped at our RLIMIT_NOFILE, so we can't get
EINVAL from poll(2).
Simon McVittie [Mon, 24 Jan 2011 14:38:13 +0000 (14:38 +0000)]
DBusLoop: inline add_callback, remove_callback into their callers
The watch and timeout code paths will diverge completely when we change
WatchCallback * to just be a DBusWatch *, removing the benefit of having
common code.
Simon McVittie [Tue, 25 Jan 2011 12:37:01 +0000 (12:37 +0000)]
DBusLoop: move OOM flag in watches inside the DBusWatch
This will eventually let us maintain a DBusPollFD[] of just the active
watches, between several iterations. The more immediate benefit is that
WatchCallback can go away, because it only contains a refcount, a
now-useless type, and a watch that already has its own refcount.
This doesn't take any more memory for DBusWatch when not using DBusLoop
(e.g. in client/service code or bindings), because we're just using more
bits in an existing bitfield.