[evolution-data-server] EGdbusTemplates: Address crash on operation cancel

[Milan Crha <mcrha src gnome org>]

commit 13cdeb66cbe2db348991f3384307fd85df8cf68f
Author: Milan Crha <mcrha redhat com>
Date:   Fri Jan 25 17:21:31 2013 +0100

    EGdbusTemplates: Address crash on operation cancel
    
    In situations when a synchronous operation was cancelled, but the response
    was already piled in main context the client could receive two replies, one
    from the reply, the other from the cancelled operation, effectively accessing
    invalid memory in the second response. This may address also other similar
    situations caused by cancelled operations.

 libedataserver/e-gdbus-templates.c |  104 +++++++++++++++++++++++++++++++-----
 1 files changed, 91 insertions(+), 13 deletions(-)
---
diff --git a/libedataserver/e-gdbus-templates.c b/libedataserver/e-gdbus-templates.c
index 60306be..ef3e476 100644
--- a/libedataserver/e-gdbus-templates.c
+++ b/libedataserver/e-gdbus-templates.c
@@ -844,6 +844,7 @@ e_gdbus_complete_sync_method_uint (gpointer object,
 
 typedef struct _AsyncOpData
 {
+	gint ref_count;
 	EGdbusAsyncOpKeeper *proxy;
 	guint opid;
 
@@ -871,27 +872,37 @@ async_op_data_free (AsyncOpData *op_data)
 
 	g_return_if_fail (op_data != NULL);
 
+	pending_ops = e_gdbus_async_op_keeper_get_pending_ops (op_data->proxy);
+
 	if (op_data->cancel_idle_id) {
 		GError *error = NULL;
 
 		g_source_remove (op_data->cancel_idle_id);
 		op_data->cancel_idle_id = 0;
 
+		if (pending_ops)
+			g_hash_table_remove (pending_ops, GUINT_TO_POINTER (op_data->opid));
+
 		if (!e_gdbus_async_op_keeper_cancel_op_sync (op_data->proxy, op_data->opid, NULL, &error)) {
 			g_debug ("%s: Failed to cancel operation: %s\n", G_STRFUNC, error ? error->message : "Unknown error");
 			g_clear_error (&error);
 		}
+	} else if (pending_ops) {
+		g_hash_table_remove (pending_ops, GUINT_TO_POINTER (op_data->opid));
 	}
 
 	if (op_data->cancellable) {
-		if (op_data->cancel_id)
+		if (op_data->cancel_id) {
 			g_cancellable_disconnect (op_data->cancellable, op_data->cancel_id);
+			op_data->cancel_id = 0;
+		}
 		g_object_unref (op_data->cancellable);
+		op_data->cancellable = NULL;
 	}
 
-	pending_ops = e_gdbus_async_op_keeper_get_pending_ops (E_GDBUS_ASYNC_OP_KEEPER (op_data->proxy));
-	if (pending_ops)
-		g_hash_table_remove (pending_ops, GUINT_TO_POINTER (op_data->opid));
+	if (!g_atomic_int_dec_and_test (&op_data->ref_count))
+		return;
+
 	g_object_unref (op_data->proxy);
 
 	switch (op_data->result_type) {
@@ -919,6 +930,7 @@ async_op_complete (AsyncOpData *op_data,
 
 	g_return_if_fail (op_data != NULL);
 
+	g_atomic_int_inc (&op_data->ref_count);
 	simple = g_simple_async_result_new (G_OBJECT (op_data->proxy), op_data->async_callback, op_data->async_user_data, op_data->async_source_tag);
 	g_simple_async_result_set_op_res_gpointer (simple, op_data, (GDestroyNotify) async_op_data_free);
 	if (error)
@@ -932,13 +944,43 @@ async_op_complete (AsyncOpData *op_data,
 	g_object_unref (simple);
 }
 
+typedef struct _CancelData
+{
+	EGdbusAsyncOpKeeper *proxy;
+	guint opid;
+	AsyncOpData *op_data;
+} CancelData;
+
+static void
+cancel_data_free (gpointer ptr)
+{
+	CancelData *cd = ptr;
+
+	if (!cd)
+		return;
+
+	g_object_unref (cd->proxy);
+	g_free (cd);
+}
+
 static gboolean
 e_gdbus_op_cancelled_idle_cb (gpointer user_data)
 {
-	AsyncOpData *op_data = user_data;
+	CancelData *cd = user_data;
+	AsyncOpData *op_data;
+	GHashTable *pending_ops;
 	GCancellable *cancellable;
 	GError *error = NULL;
 
+	g_return_val_if_fail (cd != NULL, FALSE);
+
+	pending_ops = e_gdbus_async_op_keeper_get_pending_ops (cd->proxy);
+	if (pending_ops && !g_hash_table_lookup (pending_ops, GUINT_TO_POINTER (cd->opid))) {
+		/* got served already */
+		return FALSE;
+	}
+
+	op_data = cd->op_data;
 	g_return_val_if_fail (op_data != NULL, FALSE);
 
 	cancellable = op_data->cancellable;
@@ -961,12 +1003,19 @@ static void
 e_gdbus_op_cancelled_cb (GCancellable *cancellable,
                          AsyncOpData *op_data)
 {
+	CancelData *cd;
+
 	g_return_if_fail (op_data != NULL);
 	g_return_if_fail (op_data->cancellable == cancellable);
 
+	cd = g_new0 (CancelData, 1);
+	cd->proxy = g_object_ref (op_data->proxy);
+	cd->opid = op_data->opid;
+	cd->op_data = op_data;
+
 	/* do this on idle, because this callback should be left
 	 * as soon as possible, with no sync calls being done */
-	op_data->cancel_idle_id = g_idle_add (e_gdbus_op_cancelled_idle_cb, op_data);
+	op_data->cancel_idle_id = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, e_gdbus_op_cancelled_idle_cb, cd, cancel_data_free);
 }
 
 static void
@@ -1370,9 +1419,21 @@ e_gdbus_proxy_sync_ready_cb (GObject *proxy,
                              GAsyncResult *result,
                              gpointer user_data)
 {
-	SyncOpData *sync_data = user_data;
+	gint sync_opid = GPOINTER_TO_INT (user_data);
+	gchar *sync_opid_ident;
+	SyncOpData *sync_data;
+
+	sync_opid_ident = g_strdup_printf ("EGdbusTemplates-SyncOp-%d", sync_opid);
+	sync_data = g_object_get_data (proxy, sync_opid_ident);
+	g_free (sync_opid_ident);
+
+	if (!sync_data) {
+		/* already finished operation; it can happen when the operation is cancelled,
+		   but the result is already waiting in an idle queue.
+		*/
+		return;
+	}
 
-	g_return_if_fail (sync_data != NULL);
 	g_return_if_fail (sync_data->flag != NULL);
 
 	switch (sync_data->out_type) {
@@ -1415,12 +1476,17 @@ e_gdbus_proxy_call_sync (GDBusProxy *proxy,
                          guint out_type,
                          gpointer out_value)
 {
+	static volatile gint sync_op_counter = 0;
+	gint sync_opid;
+	gchar *sync_opid_ident;
 	SyncOpData sync_data = { 0 };
 
 	g_return_val_if_fail (proxy != NULL, FALSE);
 	g_return_val_if_fail (start_func != NULL, FALSE);
 	g_return_val_if_fail (finish_func != NULL, FALSE);
 
+	g_object_ref (proxy);
+
 	switch (out_type) {
 	case E_GDBUS_TYPE_VOID:
 		sync_data.finish_func.finish_void = finish_func;
@@ -1443,6 +1509,7 @@ e_gdbus_proxy_call_sync (GDBusProxy *proxy,
 		break;
 	default:
 		g_warning ("%s: Unknown 'out' E_GDBUS_TYPE %x", G_STRFUNC, out_type);
+		g_object_unref (proxy);
 		return FALSE;
 	}
 
@@ -1450,30 +1517,37 @@ e_gdbus_proxy_call_sync (GDBusProxy *proxy,
 	sync_data.error = error;
 	sync_data.out_type = out_type;
 
+	sync_opid = g_atomic_int_add (&sync_op_counter, 1);
+	sync_opid_ident = g_strdup_printf ("EGdbusTemplates-SyncOp-%d", sync_opid);
+	g_object_set_data (G_OBJECT (proxy), sync_opid_ident, &sync_data);
+
 	switch (in_type) {
 	case E_GDBUS_TYPE_VOID: {
 		EGdbusCallStartVoid start = start_func;
-		start (proxy, cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
+		start (proxy, cancellable, e_gdbus_proxy_sync_ready_cb, GINT_TO_POINTER (sync_opid));
 	} break;
 	case E_GDBUS_TYPE_BOOLEAN: {
 		EGdbusCallStartBoolean start = start_func;
-		start (proxy, * ((gboolean *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
+		start (proxy, * ((gboolean *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, GINT_TO_POINTER (sync_opid));
 	} break;
 	case E_GDBUS_TYPE_STRING: {
 		EGdbusCallStartString start = start_func;
-		start (proxy, (const gchar *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
+		start (proxy, (const gchar *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, GINT_TO_POINTER (sync_opid));
 	} break;
 	case E_GDBUS_TYPE_STRV: {
 		EGdbusCallStartStrv start = start_func;
-		start (proxy, (const gchar * const *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
+		start (proxy, (const gchar * const *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, GINT_TO_POINTER (sync_opid));
 	} break;
 	case E_GDBUS_TYPE_UINT: {
 		EGdbusCallStartUint start = start_func;
-		start (proxy, * ((guint *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
+		start (proxy, * ((guint *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, GINT_TO_POINTER (sync_opid));
 	} break;
 	default:
 		g_warning ("%s: Unknown 'in' E_GDBUS_TYPE %x", G_STRFUNC, in_type);
 		e_flag_free (sync_data.flag);
+		g_object_set_data (G_OBJECT (proxy), sync_opid_ident, NULL);
+		g_free (sync_opid_ident);
+		g_object_unref (proxy);
 		return FALSE;
 	}
 
@@ -1502,8 +1576,12 @@ e_gdbus_proxy_call_sync (GDBusProxy *proxy,
 		/* is called in a dedicated thread */
 		e_flag_wait (sync_data.flag);
 	}
+	g_object_set_data (G_OBJECT (proxy), sync_opid_ident, NULL);
 	e_flag_free (sync_data.flag);
 
+	g_free (sync_opid_ident);
+	g_object_unref (proxy);
+
 	return sync_data.finish_result;
 }