[Libreoffice-commits] D-Bus is not thread safe

[Stephan Bergmann]

With the below commit I do not get any spurious D-Bus-related crashes 
during startup of "soffice --impress" of my (--enable-avahi 
--enable-dbus) Linux build any more, but I still get

> warn:sdremote.bluetooth:10777:7:sd/source/ui/remotecontrol/BluetoothServer.cxx:174: invalid type of reply to DefaultAdapter: '115'

on every startup of "soffice --impress", so it smells like the D-Bus 
activity in sd::BluetoothServer::run still does not play along nicely 
with the concurrent D-Bus activity in sd::AvahiNetworkService::setup.

Stephan

On 10/14/2013 06:01 PM, Stephan Bergmann wrote:
> commit 0f1357200ee710e84389e927859eac75d24323d3
> Author: Stephan Bergmann <sbergman at redhat.com>
> Date:   Mon Oct 14 17:46:57 2013 +0200
>
>      D-Bus is not thread safe
>
>      ...so it could happen that both the main thread at
>
>      > internal_bus_get
>      > dbus_bus_get_private
>      > avahi_dbus_bus_get
>      > avahi_client_new
>      > sd::AvahiNetworkService::setup
>      > sd::DiscoveryService::DiscoveryService
>      > sd::DiscoveryService::setup
>      > SdDLL::RegisterRemotes
>      [...]
>
>      as well as the thread
>
>      > internal_bus_get
>      > dbus_bus_get
>      > dbusConnectToNameOnBus
>      > sd::BluetoothServer::run
>      > threadFunc
>      > osl_thread_start_Impl
>      > start_thread
>
>      spawned from the main thread at
>
>      > sd::BluetoothServer::setup
>      > sd::RemoteServer::setup
>      > SdDLL::RegisterRemotes
>      [...]
>
>      are in D-Bus's internal_bus_get simultaneously (with disastrous consequences,
>      like SEGV) despite the _DBUS_LOCK(bus) there, unless you previously called
>      dbus_threads_init_default.  (Which the D-Bus documentation makes you believe can
>      be called from multiple threads, though a look at the implemenation makes it
>      clear that it really should be called from the main thread before any other
>      threads are created---which we still don't do; oh my.)
>
>      Other places that (indirectly) use D-Bus (tubes/source/file-transfer-helper.c,
>      vcl/generic/fontmanager/fontconfig.cxx, vcl/unx/gtk/window/gtksalframe.cxx might
>      need this, too.
>
>      Change-Id: I912829c615b46b05a89c07bd044b04f1e5f5e7ba
>
> diff --git a/sd/source/ui/remotecontrol/AvahiNetworkService.cxx b/sd/source/ui/remotecontrol/AvahiNetworkService.cxx
> index 8fc4eb7..34b94a3 100644
> --- a/sd/source/ui/remotecontrol/AvahiNetworkService.cxx
> +++ b/sd/source/ui/remotecontrol/AvahiNetworkService.cxx
> @@ -8,6 +8,7 @@
>    */
>   #include <time.h>
>   #include <iostream>
> +#include <new>
>   #include <stdlib.h>
>   #include <assert.h>
>
> @@ -20,6 +21,8 @@
>   #include <avahi-common/timeval.h>
>   #include <avahi-common/thread-watch.h>
>
> +#include <dbus/dbus.h>
> +
>   #include <sal/log.hxx>
>
>   #include "AvahiNetworkService.hxx"
> @@ -149,6 +152,13 @@ static void client_callback(AvahiClient *c, AvahiClientState state, AVAHI_GCC_UN
>   }
>
>   void AvahiNetworkService::setup() {
> +    // Avahi internally uses D-Bus, which requires the following in order to be
> +    // thread-safe (and we potentially access D-Bus from different threads in
> +    // different places of the code base):
> +    if (!dbus_threads_init_default()) {
> +        throw std::bad_alloc();
> +    }
> +
>      int error = 0;
>      avahiService = this;
>      if (!(threaded_poll = avahi_threaded_poll_new())) {
> diff --git a/sd/source/ui/remotecontrol/BluetoothServer.cxx b/sd/source/ui/remotecontrol/BluetoothServer.cxx
> index ccdf03f..1deab3c 100644
> --- a/sd/source/ui/remotecontrol/BluetoothServer.cxx
> +++ b/sd/source/ui/remotecontrol/BluetoothServer.cxx
> @@ -11,6 +11,7 @@
>
>   #include <iostream>
>   #include <iomanip>
> +#include <new>
>
>   #include <sal/log.hxx>
>
> @@ -586,6 +587,13 @@ BluetoothServer::BluetoothServer( std::vector<Communicator*>* pCommunicators )
>       mpCommunicators( pCommunicators )
>   {
>   #ifdef LINUX_BLUETOOTH
> +    // D-Bus requires the following in order to be thread-safe (and we
> +    // potentially access D-Bus from different threads in different places of
> +    // the code base):
> +    if (!dbus_threads_init_default()) {
> +        throw std::bad_alloc();
> +    }
> +
>       mpImpl.reset(new BluetoothServer::Impl());
>   #endif
>   }