Bug 29943

Created attachment 38362 [details] [review]
Give the correct type to the debug_sender global variable.

Created attachment 38363 [details] [review]
Make TpDebugSender's g_log handler thread safe

Review of attachment 38362 [details] [review]:

Sadly, yes there is a reason.

::: telepathy-glib/debug-sender.c
@@ +207,3 @@
           type, n_construct_params, construct_params);
+      debug_sender = TP_DEBUG_SENDER (retval);
+      g_object_add_weak_pointer (retval, (gpointer *) &debug_sender);

This violates ISO C99 aliasing rules, which is why it was a gpointer in the first place.

(void ** and TpDebugSender ** are not "similar enough" to be considered aliasable - void * is special but void ** is not.)

Review of attachment 38363 [details] [review]:

::: telepathy-glib/debug-sender.c
@@ +401,3 @@
+    goto ignore_message;
+
+  if (g_queue_get_length (debug_sender->priv->messages) >= DEBUG_MESSAGE_LIMIT)

This stuff needs factoring out of tp_debug_sender_add_message() into a common helper, instead of being duplicated.

@@ +494,3 @@
+  /* This could be called from any thread, but since Telepathy-GLib is not
+   * in any way thread safe, we must send it to the main loop to be
+   * processedx

"processed"

(FWIW, it's mostly dbus-glib that's not thread-safe)

Created attachment 38364 [details] [review]
updated patch

Arg, I hate strict aliasing rules, what a waste of time.

Created attachment 38365 [details] [review]
updated again

Split the function into two parts..

Also, in this case, its not only dbus-glib that was crashing, but playing around with the GQueue in TpDebugSender ;)

Review of attachment 38365 [details] [review]:

Either the patch review stuff is lying to me, or this is just your original patch back again?

Stealing this bug because we want this before telepathy-glib 0.12.0. How's this?

http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/thread-safety

Fixed in git for 0.11.15