Bug 54770 - dbus_g_thread_init() should be allowed to be called multiple times
Summary: dbus_g_thread_init() should be allowed to be called multiple times
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: GLib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on: 54972
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-11 11:01 UTC by Chow Loong Jin
Modified: 2015-02-09 16:30 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to amend documentation of dbus_g_thread_init() (1.34 KB, patch)
2012-09-11 11:01 UTC, Chow Loong Jin
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chow Loong Jin 2012-09-11 11:01:38 UTC
Created attachment 66965 [details] [review]
Patch to amend documentation of dbus_g_thread_init()

The documentation specified that dbus_g_thread_init() could only be called once,
which was untrue, as all it did was call dbus_threads_init_default() which can
be called as many times as necessary.

This amendment will allow dbus_g_thread_init() to be called from within
libraries that require multithreaded access to lidbus without violating the API.

One such example is gconf, as seen in http://bugzilla.gnome.org/659756.
Comment 1 Ray Strode [halfline] 2012-09-13 20:32:58 UTC
as discussed in https://bugzilla.gnome.org/show_bug.cgi?id=683830 the docs really should point out that you can't use libdbus before initializing threading, then initialize threading, then continue to use libdbus afterward.
Comment 2 Simon McVittie 2012-11-09 12:07:37 UTC
(In reply to comment #1)
> as discussed in https://bugzilla.gnome.org/show_bug.cgi?id=683830 the docs
> really should point out that you can't use libdbus before initializing
> threading, then initialize threading, then continue to use libdbus afterward.

Actually, this is meant to work. libdbus keeps a list of the locations of all of its fake mutexes, and when you initialize threading, libdbus goes through the list replacing them with real mutexes.

However, note that dbus-glib isn't thread-safe. Using the dbus-glib main loop glue to dispatch libdbus *might* work from threads other than the main thread; the dbus-glib specialized GType system (and everything built on it, like exported objects and DBusGProxy) certainly won't. I don't think this is worth fixing: use GDBus.

Relatedly, dbus_g_thread_init() currently ignores the return value from dbus_threads_init_default(), which is clearly wrong: it should g_error ("out of
memory") (or something) if dbus_threads_init_default() fails.
Comment 3 Ray Strode [halfline] 2012-11-09 16:55:30 UTC
I don't think the swapping of fake-mutexes to real-mutexes is threadsafe, but you certainly know the code a lot better than I do.

Though, the commit where that feature was added says this:

In this model threads can be initialized even after the D-Bus API has been used but still needs to be initialized before the second thread has been started.  Mutexes and condvar addresses are stored in the two static lists and are replaced with actuall locks when threads are initalized.
Comment 4 Simon McVittie 2013-06-17 15:51:55 UTC
(In reply to comment #3)
> I don't think the swapping of fake-mutexes to real-mutexes is threadsafe

It's not. The thing that is meant to have worked is:

* be single-threaded
* use libdbus for a while
* decide you actually wanted a second thread
* call dbus_threads_init_default() (while still single-threaded!)
* start your second thread

Caveats:

* dbus_threads_init_default() can only be called while single-threaded
* libdbus up to 1.4 had a faulty implementation of recursive mutexes, so it was never really thread-safe anyway
* even when run on a thread-safe libdbus, dbus-glib is not, in general, thread-safe

From dbus-1.7.4, dbus_threads_init_default() is meant to be OK to call from any thread. I consider this to be somewhat experimental.

From dbus-1.7.6, dbus_threads_init_default() will hopefully be unnecessary. See Bug #54972.
Comment 5 Simon McVittie 2014-09-16 12:40:27 UTC
(In reply to comment #4)
> From dbus-1.7.6, dbus_threads_init_default() will hopefully be unnecessary.
> See Bug #54972.

I've proposed a patch on Bug #64214 that changes the documentation of dbus_g_thread_init() to not imply that it can only be called once, but also explicitly document that the object layer is not thread-safe (which has, AIUI, always been true). Please review.
Comment 6 Simon McVittie 2015-02-09 16:30:04 UTC
(In reply to Simon McVittie from comment #5)
> I've proposed a patch on Bug #64214 that changes the documentation of
> dbus_g_thread_init() to not imply that it can only be called once, but also
> explicitly document that the object layer is not thread-safe (which has,
> AIUI, always been true). Please review.

Fixed in 0.104.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.