I've recently been investigating a crash which was traced to
interactions between libdbus, glib-networking and pacrunner's libproxy
shim, and thought I should open a bug or something. I can't find a bug
tracker for pacrunner, so, mailing list it is...
Background
----------
Based on cursory inspection of its source code, "the real libproxy"
appears to be designed to be thread-safe. The plugin in glib-networking
that uses libproxy as a backend for GIO proxy resolution calls into
libproxy from threads: in particular, if you are resolving the proxy for
two URLs, glib-networking can be blocking in
px_proxy_factory_get_proxies() from more than one thread at the same
time. (This is not theoretical; I've seen it happen in gdb.)
pacrunner's shim implementation of libproxy uses libdbus, wrapping a
private DBusConnection. However, libdbus has a long-standing design
flaw, rooted in its efforts to be widely portable: it is not thread-safe
unless an application explicitly tells it to be thread-safe (at which
point it initializes locks for global data structures, etc.). In an
ideal world, yes this should be fixed (and I've developed patches that I
think do that), but the changes are not trivial, and there are several
years' worth of libdbus versions that are not thread-safe by default.
The way an application tells libdbus to be thread-safe is:
if (!dbus_threads_init_default ())
fatal ("out of memory"); /* or some better error-handling */
However, dbus_threads_init_default() is not itself thread-safe: if
you're pathologically unlucky, two threads calling
dbus_threads_init_default() simultaneously could result in crashes! Its
documented correct usage is basically "call it before you start your
second thread", which is not something that a library like libproxy can
do without help from its host application. Again, this is undeniably a
design flaw, but fixing it is not trivial.
(The pacrunner *daemon* does use dbus_threads_init_default() correctly,
i.e. from main() before it does anything significant.)
Solutions
---------
The ideal solution for the future is fixing those design flaws in
libdbus 1.7, for which I have patches up for review at
<https://bugs.freedesktop.org/show_bug.cgi?id=54972> (reviews from
multi-threading experts would be extremely welcome); but that can't
retroactively fix older versions, so something else is still needed.
One possible solution is to use a better library, written with the
benefit of hindsight (not replicating libdbus' mistakes). GIO's GDBus is
LGPL, C, thread-safe by default since 2.32, and appears to be
well-designed for multi-threaded situations, although it does add a
library dependency (libgio-2.0).
Another possible solution is to document pacrunner's libproxy as an
imperfect emulation of "the real libproxy" in that it is not
thread-safe, and warn its users to audit their applications to ensure
that they avoid using libproxy from non-main threads unless the host
application is known to have called dbus_threads_init_default() on
startup. In the case of glib-networking, which does use threads and
cannot generally assume that that dbus_threads_init_default() has been
called, that would probably mean not compiling its libproxy module
against the shim libproxy, and instead using a new "pacrunner" module
that makes the same D-Bus calls using GDBus.
Another possibility is for the shim libproxy to call
dbus_threads_init_default() during px_proxy_factory_new(), in the hope
that it is not racing with another thread - this is itself neither safe
nor correct, but it seems likely to be less bad, since two threads
calling px_proxy_factory_new() simultaneously seems less likely than two
threads calling px_proxy_factory_get_proxies() simultaneously. If you do
this, please consult the libdbus source code to confirm what can go
wrong and how likely it is.
(If my first few patches from
<https://bugs.freedesktop.org/show_bug.cgi?id=54972> are reviewed and
merged, in subsequent releases calling dbus_threads_init_default() from
arbitrary threads at arbitrary times will become a safe, correct and
documented thing to do; if the whole patch series is merged, it will
also become unnecessary.)
Regards,
S
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
