Bug 65296

Summary:	Gabble should initialize libdbus for thread-safety
Product:	Telepathy	Reporter:	Simon McVittie <smcv>
Component:	gabble	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	will, xclaesse
Version:	unspecified	Keywords:	patch
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	[Gabble] Initialize libdbus for thread-safety [Gabble] patch v2 [Rakia] Initialize libdbus for thread safety [Haze] Initialize libdbus for thread-safety

Description Simon McVittie 2013-06-03 11:55:34 UTC

Created attachment 80220 [details] [review]
[Gabble] Initialize libdbus for thread-safety

libdbus is not thread-safe by default. This is a long-standing design
flaw (<https://bugs.freedesktop.org/show_bug.cgi?id=54972>).

We call into GIO, which calls into glib-networking, which can
(at least in recent versions) invoke libproxy in a thread. libproxy
apparently has a Network-Manager plugin, which uses libdbus in that
thread; meanwhile, we use libdbus in the main thread and everything
goes badly for us.

(It's possible that this crash is only reproducible with broken
connectivity: I wrote this patch on a train, with intermittent
mobile broadband coverage.)

In libdbus < 1.7.4, libraries cannot safely initialize libdbus for
multi-threading, because that initialization is not itself
thread-safe (!); in particular, glib-networking cannot safely initialize
libdbus. So, we have to do it.

I have written patches to make libdbus thread-safe-by-default, but
they haven't all been reviewed and merged yet, and in any case they
won't be in a stable libdbus until 1.8. Until then, each application
has to discover and fix this bug individually.

---

The rest of the CMs should do this too - either they use GIO now, or they should eventually.

One day, Bug #54972 will make this silly dance unnecessary. Reviews over there would be extremely welcome.

Comment 1 Simon McVittie 2013-06-03 11:56:18 UTC

The patch given here assumes a couple of things from Bug #65296, but if this one is applied first, the conflicts are trivial.

Comment 2 Simon McVittie 2013-06-03 13:21:58 UTC

Created attachment 80225 [details] [review]
[Gabble] patch v2

Attachment #80220 [details] didn't check whether d_t_i_d() had worked. (It always should - it's only documented to fail on out-of-memory.)

Comment 3 Simon McVittie 2013-06-03 14:25:05 UTC

Created attachment 80231 [details] [review]
[Rakia] Initialize libdbus for thread safety

Comment 4 Xavier Claessens 2013-06-04 10:11:38 UTC

+1, maybe similar patch can be added to empathy_init().

Comment 5 Simon McVittie 2013-06-04 11:27:57 UTC

Created attachment 80286 [details] [review]
[Haze] Initialize libdbus for thread-safety

---

Haze's coding style is a bit different. Otherwise the same patch.

Comment 6 Simon McVittie 2013-06-04 11:50:45 UTC

Comment on attachment 80225 [details] [review]
[Gabble] patch v2

Gabble fixed in git for 0.17.5

Comment 7 Simon McVittie 2013-06-04 11:51:10 UTC

Comment on attachment 80231 [details] [review]
[Rakia] Initialize libdbus for thread safety

Rakia fixed in git for 0.7.5

Comment 8 Will Thompson 2013-06-23 10:43:23 UTC

Comment on attachment 80286 [details] [review]
[Haze] Initialize libdbus for thread-safety

Review of attachment 80286 [details] [review]:
-----------------------------------------------------------------

Obviously this is fine, merge away.

Comment 9 Will Thompson 2013-06-23 10:45:51 UTC

Comment on attachment 80286 [details] [review]
[Haze] Initialize libdbus for thread-safety

merged as http://cgit.freedesktop.org/telepathy/telepathy-haze/commit/?id=cbe583bce4ceec03caf9b2d67af0ea6ca3ea8818

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.