- moved common code into base class (prepare/send) reps. .cpp file (CheckError())
- retrieve D-Bus error description from error message and use it as part of
the C++ exception description, in addition to the error name (only error
name was used before)
- add call operators with up to 10 parameters
Added methods and classes for setting up direct D-Bus communication.
Only implemented for the non-GIO GDBus at the moment.
The callback must not throw exceptions because it is invoked from
plain C code.
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.
After removing a watch, destroying the glib source once more accessed
the now invalid "watch" pointer in finalize_watch(). Found in D-Bus
server mode after closing one connection.
The GSource was kept running and continued to use the freed
connection. Must use g_source_destroy() instead of g_source_unref() to
remove the source from the main loop in addition to decreasing its
ref count.
free_watch() is the right place to free WatchData, not
finalize_watch(), because the call sequence is free_watch() ->
g_source_destroy(source) -> finalize_watch().
Closing an unshared connection in free_connection() is too late,
because that is the destructor callback which is only called after
the last reference was already dropped. For the same reason the
dbus_connection_unref() there was wrong.
That code was never called in SyncEvolution because all connections
had their reference count increased one time to often when assigning
to DBusConnectionPtr. Fix will be committed separately. The problems
in this commit were found after fixing that problem and introducing
temporary connections in a D-Bus server, because then the faulty code
was called.
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.
When a method call caused the interface of the method to be removed,
GDBus used invalid memory after the method returned. It must be
prepared for that and cache the relevant information (the method flags
in this case).
If the watch was never activated, m_tag was never set, which led to an
invalid memory read in the destructor. Also be more careful with using
the connection and better check that it still exists.
dbus_message_iter_get_signature() returns a newly allocated string.
Freeing that now with a smart pointer. SmartPtr.h is not used to
avoid dependencies on the rest of SyncEvolution - just in case that
someone finds this useful.
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.
Recent glib deprecates the direct inclusion of some of its headers,
in favor of including glib.h. Doing that here whenever possible. gi18n.h
still needs to be included directly (otherwise it doesn't compile
on Debian Testing with glib 2.28).
Not sure about gio. I'm keeping the gio/ includes because they
are relevant on platforms where gio was not yet part of glib.
Somehow clang is sensitive to the order of definitions when using
boost::intrusive_ptr. This broke during the D-Bus server reorganization.
Putting the intrusive_ptr_* methods into the global name space instead
of boost works with clang and gcc and avoids the issue.
All but toplevel Makefile.am are replaced with their non-recursive
counterparts. The generation of configure.in was removed (and thus
configure-{pre,post}.in are also removed) in favor of configure.ac
and m4 macros adding backend specific configure parts.
Version number is generated like in old build system.
There are still many things to improve, but for now there are no
immediate regressions. AUTOTOOLS-TODO contains a list of possible
improvements and fixes. AUTOTOOLS-TESTING contains what was tested
with current build system (configure flags, make options).
The gdbus utility library crashed here if interface->properties was NULL:
for (property = interface->properties; property->name; property++)
^^^^^^^^^^^^^^
Fixed by checking property for non-NULL in the for loop.
g++ 4.6 complains about the unused assignment. Probably this boolean
result needs to be checked. But as GDBus will be replaced soon anyway,
don't bother now.
Fedora now includes a shared libgdbus under /usr/lib. This leads to a
conflict based on the *name* of the library, not just because of
*symbols* (which was resolved earlier).
Therefore this patch renames the version that SyncEvolution depends on
to libgdbusyncevo. All SyncEvolution patches were already sent
upstream, but it is uncertain whether libgdbus will be see further
releases. Patches to make SyncEvolution use a suitable system libgdbus
library are welcome.
clang 2.8 compiles SyncEvolution + Synthesis faster than g++ 4.4.5
(3:40min instead of 4:10min on my laptop) and produces more useful
error reports. This patch fixes the code so that it compiles cleanly
with clang when using "-Wall -Werror -Wno-unknown-pragmas". Note that
clang 2.6 (Debian Squeeze) goes into an infinite recursion compiling
code using gdbus-cxx-bridge.h and dies eventually with a stack
overflow - can't be used.
Changes necessary for clang:
- eptr pointer referencing ambiguous, use *x.get() instead
- boost::intrusive_ptr* must be defined before code using it
- two-phase template checking requires explicitly specifying
members in base classes
- name clashes with plain C structs (DBusServer, DBusWatch) are
an error and need to be avoided (done with namespaces GDBusCXX and
SyncEvo)
- floats cannot be inline constants
- unused methods in local classes are warned about (left() in SyncML.cpp)
A lot of the code was roled out explicitly. This patch replaces
that with an approach similar to iostreams: remember context, then
work on one parameter at a time.
Because the decision what to do with a certain method parameter
depends on the type of the parameter, that type must be used to pick
between Get/Set classes which skip resp. work on the parameter.
Found this one when adding a method with five parameters. The
boost::bind() call had the wrong number of parameters for that case
and thus compilation failed.
This reverts commit ec7eb74366.
With the symbol prefix renamed from gdbus into bdbus, we can install
the lib into /usr/lib/syncevolution again, to get it shared between
the "syncevolution" and "syncevo-dbus-server" binaries.
The previous solution of linking Bluex gdbus statically was not
enough. On Fedora 14, compile errors due to the glib header files
being pulled in indirectly appeared.
This patch does a global search/replace which changes the
"gdbus" (GLib D-Bus) prefix into "bdbus" (Bluez D-Bus). For the
record:
perl -pi 's/g_dbus_/b_dbus_/g; s/GDBus/BDBus/g; s/GDBUS/BDBUS/g' ...
SyncEvolution on FC 13 crashes because it conflicts with a different
system version of the gdbus library. Always linking statically fixes
this.
There never was an official upstream release of gdbus, so sharing the
implementation does not work.
The issue is to access an invalid memory. The reason is that:
DBusClientCall creates a CallbackData for each dbus call. The
CallbackData itself references the DBusClientCall object. This
object is used to get returned value once the dbus call returns.
However, the typical usage is that DBusClientCall object
is a local variable allocated on the stack. So it is invalid after
existing the stack frame. If using this object in the dbus call
handler, cause a severe problem of reading invalid memory.
The solution is to store an always alive connection object in the
CallbackData instead of DBusClientCall object.
When passwords in the config are not set, the daemon asks them from
clients like command line. Command line should react and let
user input passwords and then send responses to the daemon.
Command line listens to 'Server.InfoRequest' signal and only handles
password requests from sessions created for command line in the daemon.
If command line dies in the process of inputting passwords, the daemon
always waits for it so expand current timeout mechanism into this
process. Otherwise, the daemon waits permanently.
Add a 'DBusWatch' to watch daemon and exit the process when the
daemon has gone.
Also find that current 'DBusClientCall' related classes are not
efficient and the code is reduntant. Re-design and implement the
inheritance.
This patch started with the goal of changing the uint level into
a string. The motivation for that was that the numeric constants
would have to be documented (which they weren't) and prevent
us from easily changing the Logger::Level later on.
I did change them once today (see previous commit about reordering the
SHOW level) and really had the problem that a not-restarted
syncevo-dbus-server couldn't talk to a recompiled client.
Using strings avoids that problem.
While changing uint to string I had to make the change twice, once in
an entirely unnecessary new and the pointer definition. Replaced the
shared_ptr with normal members, throughout the code.
Then I wondered about the use of the () operator on these signal
watches. At the point of use it was not obvious what that did. Turns
out that this activates the watch. In contrast to making a remote
function call or emitting a signal, I find the use of the operator
inappropriate for watches. Replaced with an explicit activate()
method.
While looking at the implementation, I found that this operator and
destructors had been duplicated. Instead of fixing it in all copies, I
moved the common code into a SignalWatch<> base template,
parameterized with the type of callback that it deals with.
Implement cmdline with support of dbus server. To enable cmdline
with dbus server, use the option '--use-daemon yes/no' in case that
you enable dbus service when configuration.
In a typical scenario, a new session is created for the purpose of
execution of arguments. It is scheduled with other sessions but with
a highest priority. Once it becomes active, command line call
'Session.Execute', a newly added method to execute command line
arguments.
The config name of a session should be known for dbus clients like
command line. A new property 'configName' is added in the properties
when calling 'Session.GetConfig'.
CTRL-C handling are processed once executing a real sync to dbus
server. It is mapped to invoke 'Session.Suspend' and 'Session.Abort'.
The meaning of '--enable-dbus-service' is expanded accordingly.
'--status' without server means printing all running session in the
dbus server.
'--monitor' could accept an optional config name. If one is given,
only attach to a session of that config, otherwise print an error.
If none is given, pick the first.
Now we print
[ERROR] g_dbus_setup_bus() failed - server already running?
instead of just
[ERROR] g_dbus_setup_bus() failed
Starting the server twice was the most common reason for the problem.
Unfortunately we cannot be sure, because g_dbus_setup_bus() doesn't set
an error.
Calling dbus_connection_unref() alone is not enough for a private
connection, libdbus wants us to call dbus_connection_close() first.
One situation where this was observed was starting the SyncEvolution
D-Bus server twice (MB #9991).
It is the responsibility of the g_dbus_setup_bus() caller to do the
close once the function returns, but inside it, gdbus must do it.
Improvement to add signal receivers for signals sent
from bluez. Thus, syncevo-dbus-server could detect
bluetooth devices changed dynamically.
4 signals from bluez are listened:
org.bluez.Manager: AdapterChanged
org.bluez.adapter: DeviceCreated, DeviceRemoved
org.bluez.device: PropertyChanged
To support listening signals from other services,
add support in gdbus cxx bridge.
GetConfigs returns related templates to the paired bluetooth
devices with specific scores:
1)Query against the underlying template system via device names
got from Bluez and create a list of templates for each device. Since
there might be many devices and one template from system might be
for more than one device. Thus the template names returned to clients
have to be changed. So the rule to change these templates is:
Bluetooth_<mac address of device>_<seq number>, where seq number enumerates
the templates created for this device.
2)GetConfig would get the template based the cached templates info.
Each template matched for devices is added 4 properties:
description - the description for the template
score - the calculated score based on the device name and template
deviceName - the device name that the template is for
fingerPrint - the fingerPrint of the template, used for dbus clients
to re-match with user input device info
Also, the 'syncURL' will be replaced with bluetooth mac address for
these templates.
Add one callback for each interface(DBusObjectHelper):
Each DBusObjectHelper sub-class could register its own
callback function via this parameter. The callback is
eventually called before each method function is called.
A new function pointer 'callback' is added in the InterfaceData.
The callback function is per-interface. It is set together
with the registration of interface. It provides a mechanism to let
gdbus users do something before the registered methods are called.
It must be called *before* any method of an interface is called if set.
The reason is that the method might un-register the interface so
InterfaceData and its user data might be destroyed.
The user data transferred to the callback function is the 'user_data'
member in the InterfaceData set by users of gdbus.
The main purpose of this callback is that it is wanted to track
whether the dbus server is still in use.
Implement the info exchange mechanism when the dbus server wants
any information that only dbus clients can provide.
The dbus server uses the class 'InfoReq' to handle the requested
information from it and get response from dbus clients. The main
processing logic is referred to dbus api document.
To allow multiple info requests, the DBusServer uses a map to manage
all pending info requests and dispatches responses to the corresponding
info request. Also DBusServer.createInfoReq() is provided to create
info requests.
For InfoRequest callers, A series of methods in 'InfoReq' are provided:
check - check whether the required response is gotten
cancel - cancel the info request
wait - wait until timeout, abort or suspend
getResponse - get the response result provided by dbus clients.
All methods here are asynchronous except 'wait', which waits for
response explicitly.
Out-of-memory errors were not handled correctly:
- The code using realloc() might have leaked a string.
- A static pointer was used to avoid further memory
allocation errors, but the caller then would have
freed it => try strdup() and check result in caller
instead.
This change was necessary because it turned out that using
g_dbus_setup_bus() and later dbus_g_bus_get() leads to problems
(assertion about watch data on Moblin 2.1, CRITICAL warning
and possibly other issues on Debian Lenny).
It seems that sharing a DBusConnection between different layers on top
of libdbus is either not supported or incorrectly implemented, at
least in glib-dbus.
The problem was found in SyncEvolution when using a libecal/ebook
which call D-Bus under the hood (Moblin Bugzilla #8460). SyncEvolution
has no control over those calls, therefore making the connection used
by the syncevo-dbus-server private was the easier alternative.
