Raising exceptions via throwError() looses the original source code
location information. Fixing that by explicitly passing that
information as additional parameter, created with the preprocessor
macro SE_HERE.
Several files included the complex SyncContext.h only because needed
throwError(). A better place for the revised implementation is the
Exception class which used to be in util.h, now Exception.h.
Simplifying the include statements exposed indirect include
dependencies and removed "using namespace std" from the compilation of
some source files which happened to rely on it. We want to get rid of
that name space polution, so fix the code instead of adding it back.
When using GIO, it is possible to avoid the DBusServer listening on a
publicly accessible address. Connection setup becomes more reliable,
too, because the D-Bus server side can detect that a child died
because the connection will be closed.
When using libdbus, the traditional server/listen and client/connect
model is still used. GDBus GIO mimicks the same API to minimize
changes in ForkExec. The API gets simplified to support both
approaches:
- Choosing an address is no longer possible (was only used in the
test example anyway)
- The new connection callback must be specified already when starting
the server.
The GDBus GIO implementation uses SyncEvolution utility code. Strictly
speaking, this creates a circular dependency
libsyncevolution<->libgdbus. This is solved by not linking libgdbus
against libsyncevolution and expecting executables to do that instead.
The GDBus GIO example no longer linked because of that; it gets removed
because it was of little value.
Now that we started using CLOEXEC, we might as well use it for the
permanent file descriptors created for each ForkExec instance.
When an asynchronous method call failed with an exception, the code
DBusResult got destructed first before handling the exception and then
sent a generic, uninformative error back to the caller. A test
demonstrating that for the PIM Manager SetPeer() with an invalid UID
be committed separately.
To fix this, the DBusResult must know whether it is really responsible
for replying to the method message. The method call handlers
themselves do not know whether any of their parameters is a
DBusResult, so we have to activate the automatic reply indirectly, via
a guard instance with the same life time as the handler.
When the guard destructs, it can check whether the handler has
replied, because the handler then clears the message pointers. If it
hasn't, the DBusResult becomes the owner of the message. For this to
work, an extra try/catch block is necessary. Otherwise exception
handling would still occur too late, at a time when the guard is gone
and the DBusResult has sent the generic error.
Some other classes had to be adapted to allow for this extra level of
indirection. While doing so, another bug was found.
Retrieving the interface and member used when calling a method
returned the path instead, due to a cut-and-paste error. Fixed. Not
used in practice.
The problem is in the C++ D-Bus binding. If the method that gets bound
to D-Bus returns a value, that value is ignored in the signature:
int foo() => no out parameter
It works when the method is declared as having a retval:
void foo (int &result) => integer out parameter
This problem exists for both the libdbus and the GIO D-Bus
bindings. In SyncEvolution it affects methods like GetVersions().
The fix consists of always returning a type for the R return type in
the libdbus bindings. getReturn() returned "" because the type was not
a retval. In the GIO binding, the R type was not handled at all. Also
move some common code into a helper function.
Remove global variables and allow addditional names to be owned via
DBusConnectionPtr::ownName().
g_bus_own_name_on_connection() is given shared access to a ref-counted
OwnNameAsyncData (instead of full ownership) because that way it is
guaranteed that the instance is still around while waiting for name
acquisition in undelay().
Working with obexd 0.47's Transfer object depends on path prefix
filtering of signals to be efficient. This patch adds that support in
the GDBus C++ bindings, together with some documentation on how to
use it.
It also allows relaxing the signal matching by interpreting empty
strings as "match any".
Finally it adds support for passing of sender, path, interface and
member (= signal name) information to the user of the D-Bus
binding. This is necessary because a handler with wildcards for
matching cannot know the actual values unless they are passed to the
callback explicitly. This was not possible at all before for signals
(because extracting that information relied on a message pointer,
which wasn't set while decoding signal messages) and limited to just
the sender in method implementations.
Besides in functionality, GDBus for GIO and for libdbus now also
differ slightly in their API. In order to pass meta data about a
signal into the demarshalling get() methods, they have to be stored in
ExtractArgs first and passed to get() via that struct, which is an
interface change.
Derived code which extends marshalling/demarshalling needs to be
adapted accordingly. Allowing some ifdefs elsewhere seems simpler than
adapting GDBus for libdbus. Those ifdefs can be removed once libdbus
support is no longer needed.
The patch relaxes access control to DBusObject and DBusRemoteObject
because that simplifies the code which works better with std::string.
Traditionally the sender of a signal was not checked. This is probably
related to unsolved questions like "should watching org.foo.bar follow
owner changes", but not doing any checking is just not right
either. Not fixed in this patch.
When using GIO D-Bus, sometimes messages created shortly before
shutting down were not sent. Must flush a connection explicitly.
A NOP when using libdbus.
When syncevo-dbus-server was started on demand by the D-Bus daemon,
then it registered itself with the daemon before it was ready to
serve requests. Only happened in combination with GIO D-Bus and
thus was not a problem before 1.2.99.x.
One user-visible effect was that the GTK UI did select the default
service when it was started for the first time, because it could not
retrieve that information from syncevo-dbus-server.
The fix consists of delaying the name acquisition. That gives the
caller a chance to register D-Bus objects first, before completing the
connection setup. The semantic change is that
dbus_bus_connection_undelay() must be called on new connections which
have a name (a NOP with libdbus).
This patch tries to minimize code changes. The downside of not
changing the GDBusCXX API more radically is that the bus name must be
attached to DBusConnectionPtr, where it will be copied into each
reference to the connection. Hopefully std::string is smart enough to
share the (small) data in this case. Should be solved cleaner once
libdbus support can be deprecated.
A test for auto-activation and double start of syncevo-dbus-server
is also added.
Avoid the abstract base class and implement the GDBusXX::Watch
class directly. This became necessary because moving the GIO
implementation into the .cpp resulted in strange crashes around
the virtual setCallback() method - but only when optimization
was enabled. Compiler bug?!
Simplifying the class hierarchy is the right step either way. It was
done with separate classes initially to avoid dependencies on
gdbus-cxx-bridge.h in code which just needs to interact with the base
functionality, but it turned out that no code needs that.
The static template method doesn't have to be in the header
file (doesn't depend on template parameters). Moving it into
the .cpp file to minimize compile and executable size.
Runtime work can be minimized by holding the instance
temporarily in a simpler auto pointer before moving it
into the final shared pointer.
Another reason was that compiling with optimization enabled
produced binaries which crashed in that code. However, moving
it did not help.
In contrast to the version from GDBus libdbus, GDBus GIO's
dbus_get_bus_connection() with name doesn't fail even if the name
cannot be obtained. That's because it asynchronously asks for the
name without waiting for success.
As a quick-and-dirty solution register a function which kills the
process when the name registration fails. This assumes that the
process will run as a daemon with exactly one name, where failures to
obtain that name are indeed fatal.
Strictly speaking only GDBus for libdbus really needed the "connection
lost" callback, because the "catch error when pending method call
fails" approach didn't work with it. But GDBus GIO also has a "closed"
signal. Using it is straight-forward and will allow removing the more
complex code in ForkExec.
The started GDBusServer was never stopped, thus keeping the
listening socket open. This prevented running more than 100 syncs
from the same process, because the code which allocates the listening
socket gives up searching after 100 attempts. For the sake of symmetry
do both in our GDBusServerCXX class now.
The GDBusServer instance also was never freed because our smart pointers
started by bumping the reference one time too often.
DBusCallObject and friends were not used anywhere. Removed.
DBusObject and DBusRemoteObject used pure virtual methods to let
derived classes provide information about interface, path,
destination, method and the connection. The idea behind that was that
most of these strings are static and thus do not need to be copied.
The downside is that one had to derive from these classes in order to
provide the required information. The same class could not own two
instances of the generic DBusObject to access two different
destinations. It also sprinkled the code for setting up D-Bus
operations into several different places (constructor, class definition).
Now the information is passed to the DBus[Remote]Object constructor
and stored in the classes, which are thus merely containers for the
information and thus easier to use. Users of the classes still derive
from them, to keep the change smaller, although that is no longer
necessary.
Removed plain DBusConnection pointers from the C++ interface, return
DBusConnectionPtr instead. The exception is DBusObject, which still
returns plain pointers owned by the instance (as before) to ease
integration with the underlying D-Bus library. DBUS_CONNECTION_TYPE
is not needed.
This revealed that quite some code incorrectly took another reference
to the connection when assigning to DBusConnectionPtr (assignment
always increases the refcount, only in the constructor is that
optional). As a result the private connections were never
destructed. Apparently there were some global pointers to active
connections, so that this (minor) resource leak didn't show up in
valgrind.
It also showed that the closing of the D-Bus connection never happened
properly, although libdbus requires it. The ref counting mechanism
cannot be used for this because it cannot be checked whether the last
reference is about to be dropped. The slightly hackish solution is to
designate one DBusObject as the "main owner" of the connection. When
that object destructs, it closes the connection. There might still be
some other references; they simply cannot (and shouldn't) send or
receive messages anymore.
clang 2.9 is sensitive to the order of include statements when the
boost:: namespace is used. It happened to work with Bluez GDBus, but failed
with GIO GDBus.
Let's use the global namespace because it works in all case, although
the Boost documentation makes it sound like that the boost:: namespace
should be used.
Background: Prior to this patch a C++ wrapper around Bluez gdbus, a
convienience wrapper around the low-level libdbus, was used by
syncevo-dbus-server. This patch introduces a second C++ wrapper around
the GIO GDBus implementation. The reason for introducing this second
wrapper is to move away from using the in-tree copy of Bluez gdbus and
towards a more well-maintained dbus implementation. Also, libdbus was
not designed to be thread-safe whereas GIO GDBus was.
The GIO GDBus wrapper retains the same public api as the first
one. This means the consumers of this wrapper (syncevo-dbus-server,
for example) have remained almost completely untouched. The only
exceptions are in the few case where libdbus objects where used
directly by the consuming class.
The choice of which wrapper is determined at configure time. The
option can be explicitly set using the --with-gio-gdbus and
--without-gio-gdbus flags or, if no flag is given, an adequate version
of GIO is search for. If found, the GIO GDBus wrapper is chosen.
syncEvolution has used an in-tree copy of Bluez's gdbus dbus
implentation.
With this patch there is now a configure switch which enables the use
of the GLib's GDBus dbus implmentation if the --with-gio-gdbus
configure flag is use or, if no flag is used, an adequate glib version
(2.26) is found.
This commit does *not* actually implement the use of GIO's GDBus. The
implmentation is coming soon. Thus only --without-gio-gdbus will
currently result in a working build.
