trivial: Fix two potential deadlocks in the AsProfile code

This fixes a very-rare bug in gnome-software.

1 files changed, 108 insertions, 96 deletions

diff --git a/libappstream-glib/as-profile.c b/libappstream-glib/as-profile.c
index c3e402e..034d730 100644
--- a/libappstream-glib/as-profile.c
+++ b/libappstream-glib/as-profile.c
@@ -75,6 +75,109 @@ as_profile_item_find (GPtrArray *array, const gchar *id)
 	return NULL;
 }
 
+static void
+as_profile_prune_safe (AsProfile *profile, guint duration)
+{
+	gint64 now;
+	guint i;
+	g_autoptr(GPtrArray) removal = g_ptr_array_new ();
+
+	/* find all events older than duration */
+	now = g_get_real_time () / 1000;
+	for (i = 0; i < profile->archived->len; i++) {
+		AsProfileItem *item = g_ptr_array_index (profile->archived, i);
+		if (now - item->time_start / 1000 > duration)
+			g_ptr_array_add (removal, item);
+	}
+
+	/* remove each one */
+	for (i = 0; i < removal->len; i++) {
+		AsProfileItem *item = g_ptr_array_index (removal, i);
+		g_ptr_array_remove (profile->archived, item);
+	}
+}
+
+static gint
+as_profile_sort_cb (gconstpointer a, gconstpointer b)
+{
+	AsProfileItem *item_a = *((AsProfileItem **) a);
+	AsProfileItem *item_b = *((AsProfileItem **) b);
+	if (item_a->time_start < item_b->time_start)
+		return -1;
+	if (item_a->time_start > item_b->time_start)
+		return 1;
+	return 0;
+}
+
+static void
+as_profile_dump_safe (AsProfile *profile)
+{
+	AsProfileItem *item;
+	gint64 time_start = G_MAXINT64;
+	gint64 time_stop = 0;
+	gint64 time_ms;
+	guint console_width = 86;
+	guint i;
+	guint j;
+	gdouble scale;
+	guint bar_offset;
+	guint bar_length;
+
+	/* nothing to show */
+	if (profile->archived->len == 0)
+		return;
+
+	/* get the start and end times */
+	for (i = 0; i < profile->archived->len; i++) {
+		item = g_ptr_array_index (profile->archived, i);
+		if (item->time_start < time_start)
+			time_start = item->time_start;
+		if (item->time_stop > time_stop)
+			time_stop = item->time_stop;
+	}
+	scale = (gdouble) console_width / (gdouble) ((time_stop - time_start) / 1000);
+
+	/* sort the list */
+	g_ptr_array_sort (profile->archived, as_profile_sort_cb);
+
+	/* dump a list of what happened when */
+	for (i = 0; i < profile->archived->len; i++) {
+		item = g_ptr_array_index (profile->archived, i);
+		time_ms = (item->time_stop - item->time_start) / 1000;
+		if (time_ms < 5)
+			continue;
+
+		/* print a timechart of what we've done */
+		bar_offset = (guint) (scale * (gdouble) (item->time_start -
+							 time_start) / 1000);
+		for (j = 0; j < bar_offset; j++)
+			g_print (" ");
+		bar_length = (guint) (scale * (gdouble) time_ms);
+		if (bar_length == 0)
+			bar_length = 1;
+		for (j = 0; j < bar_length; j++)
+			g_print ("#");
+		for (j = bar_offset + bar_length; j < console_width + 1; j++)
+			g_print (" ");
+		g_print ("@%04" G_GINT64_FORMAT "ms ",
+			 (item->time_stop - time_start) / 1000);
+		g_print ("%s %" G_GINT64_FORMAT "ms\n", item->id, time_ms);
+	}
+
+	/* not all complete */
+	if (profile->current->len > 0) {
+		for (i = 0; i < profile->current->len; i++) {
+			item = g_ptr_array_index (profile->current, i);
+			item->time_stop = g_get_real_time ();
+			for (j = 0; j < console_width; j++)
+				g_print ("$");
+			time_ms = (item->time_stop - item->time_start) / 1000;
+			g_print (" @????ms %s %" G_GINT64_FORMAT "ms\n",
+				 item->id, time_ms);
+		}
+	}
+}
+
 /**
  * as_profile_start:
  * @profile: A #AsProfile
@@ -123,7 +226,7 @@ as_profile_start_literal (AsProfile *profile, const gchar *id)
 
 	/* autoprune old data */
 	if (profile->autoprune_duration != 0)
-		as_profile_prune (profile, profile->autoprune_duration);
+		as_profile_prune_safe (profile, profile->autoprune_duration);
 
 	/* only use the thread ID when not using the main thread */
 	self = g_thread_self ();
@@ -136,7 +239,7 @@ as_profile_start_literal (AsProfile *profile, const gchar *id)
 	/* already started */
 	item = as_profile_item_find (profile->current, id_thr);
 	if (item != NULL) {
-		as_profile_dump (profile);
+		as_profile_dump_safe (profile);
 		g_warning ("Already a started task for %s", id_thr);
 		return NULL;
 	}
@@ -215,18 +318,6 @@ as_profile_task_free (AsProfileTask *ptask)
 	g_free (ptask);
 }
 
-static gint
-as_profile_sort_cb (gconstpointer a, gconstpointer b)
-{
-	AsProfileItem *item_a = *((AsProfileItem **) a);
-	AsProfileItem *item_b = *((AsProfileItem **) b);
-	if (item_a->time_start < item_b->time_start)
-		return -1;
-	if (item_a->time_start > item_b->time_start)
-		return 1;
-	return 0;
-}
-
 /**
  * as_profile_clear:
  * @profile: A #AsProfile
@@ -254,24 +345,9 @@ as_profile_clear (AsProfile *profile)
 void
 as_profile_prune (AsProfile *profile, guint duration)
 {
-	gint64 now;
-	guint i;
 	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&profile->mutex);
-	g_autoptr(GPtrArray) removal = g_ptr_array_new ();
-
-	/* find all events older than duration */
-	now = g_get_real_time () / 1000;
-	for (i = 0; i < profile->archived->len; i++) {
-		AsProfileItem *item = g_ptr_array_index (profile->archived, i);
-		if (now - item->time_start / 1000 > duration)
-			g_ptr_array_add (removal, item);
-	}
-
-	/* remove each one */
-	for (i = 0; i < removal->len; i++) {
-		AsProfileItem *item = g_ptr_array_index (removal, i);
-		g_ptr_array_remove (profile->archived, item);
-	}
+	g_return_if_fail (AS_IS_PROFILE (profile));
+	as_profile_prune_safe (profile, duration);
 }
 
 /**
@@ -301,73 +377,9 @@ as_profile_set_autoprune (AsProfile *profile, guint duration)
 void
 as_profile_dump (AsProfile *profile)
 {
-	AsProfileItem *item;
-	gint64 time_start = G_MAXINT64;
-	gint64 time_stop = 0;
-	gint64 time_ms;
-	guint console_width = 86;
-	guint i;
-	guint j;
-	gdouble scale;
-	guint bar_offset;
-	guint bar_length;
 	g_autoptr(GMutexLocker) locker = g_mutex_locker_new (&profile->mutex);
-
 	g_return_if_fail (AS_IS_PROFILE (profile));
-
-	/* nothing to show */
-	if (profile->archived->len == 0)
-		return;
-
-	/* get the start and end times */
-	for (i = 0; i < profile->archived->len; i++) {
-		item = g_ptr_array_index (profile->archived, i);
-		if (item->time_start < time_start)
-			time_start = item->time_start;
-		if (item->time_stop > time_stop)
-			time_stop = item->time_stop;
-	}
-	scale = (gdouble) console_width / (gdouble) ((time_stop - time_start) / 1000);
-
-	/* sort the list */
-	g_ptr_array_sort (profile->archived, as_profile_sort_cb);
-
-	/* dump a list of what happened when */
-	for (i = 0; i < profile->archived->len; i++) {
-		item = g_ptr_array_index (profile->archived, i);
-		time_ms = (item->time_stop - item->time_start) / 1000;
-		if (time_ms < 5)
-			continue;
-
-		/* print a timechart of what we've done */
-		bar_offset = (guint) (scale * (gdouble) (item->time_start -
-							 time_start) / 1000);
-		for (j = 0; j < bar_offset; j++)
-			g_print (" ");
-		bar_length = (guint) (scale * (gdouble) time_ms);
-		if (bar_length == 0)
-			bar_length = 1;
-		for (j = 0; j < bar_length; j++)
-			g_print ("#");
-		for (j = bar_offset + bar_length; j < console_width + 1; j++)
-			g_print (" ");
-		g_print ("@%04" G_GINT64_FORMAT "ms ",
-			 (item->time_stop - time_start) / 1000);
-		g_print ("%s %" G_GINT64_FORMAT "ms\n", item->id, time_ms);
-	}
-
-	/* not all complete */
-	if (profile->current->len > 0) {
-		for (i = 0; i < profile->current->len; i++) {
-			item = g_ptr_array_index (profile->current, i);
-			item->time_stop = g_get_real_time ();
-			for (j = 0; j < console_width; j++)
-				g_print ("$");
-			time_ms = (item->time_stop - item->time_start) / 1000;
-			g_print (" @????ms %s %" G_GINT64_FORMAT "ms\n",
-				 item->id, time_ms);
-		}
-	}
+	as_profile_dump_safe (profile);
 }
 
 static gboolean