[evolution-data-server/openismus-work] Revert "EGdbusTemplates: Address crash on operation cancel"

[Patrick Ohly <pohly src gnome org>]

commit 1be1ad89894b5deee6479fdb546031fd462667ea
Author: Patrick Ohly <patrick ohly intel com>
Date:   Thu Mar 28 06:33:20 2013 -0700

    Revert "EGdbusTemplates: Address crash on operation cancel"
    
    This reverts commit 7c52fbd81093c1264e3d8aa6cdcf5c8bdc7b1772.
    
    7c52fb introduced a memory leak because it indirectly creates a new
    GQuark for each synchronous method call. The string for that GQuark
    will never get freed. In addition, the table for quarks keeps growing
    and also leaks. This is what valgrind found, the strings themselves
    are still reachable and therefore not reported by valgrind.
    
    The bigger problem is that with 7c52fb, e_client_open fails with
    EDB_OPENING_ERROR = "Cannot process, book backend is opening" in the
    "SYNCEVOLUTION_NO_PIM_EDS_DIRECT=1 testpim.py
    TestContacts.testFilterStartup" test. When using DRA, the problem is
    that the client seems to use the wrong DB path.
    
    This can be either a race condition that is merely triggered by the
    change in 7c52fb or a real problem in the patch itself. Either way,
    until a solution is found it is better to not use the patch in the
    openismus-work branch.
    
    Conflicts:
        libedataserver/e-gdbus-templates.c
    
    The conflict was due to g_idle_add <-> g_idle_add_full. Resolved by using
    g_idle_add_full with NULL pointers.

 libedataserver/e-gdbus-templates.c |  104 +++++-------------------------------
 1 files changed, 13 insertions(+), 91 deletions(-)
---
diff --git a/libedataserver/e-gdbus-templates.c b/libedataserver/e-gdbus-templates.c
index d741001..8071912 100644
--- a/libedataserver/e-gdbus-templates.c
+++ b/libedataserver/e-gdbus-templates.c
@@ -844,7 +844,6 @@ e_gdbus_complete_sync_method_uint (gpointer object,
 
 typedef struct _AsyncOpData
 {
-       gint ref_count;
        EGdbusAsyncOpKeeper *proxy;
        guint opid;
 
@@ -872,37 +871,27 @@ 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;
        }
 
-       if (!g_atomic_int_dec_and_test (&op_data->ref_count))
-               return;
-
+       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));
        g_object_unref (op_data->proxy);
 
        switch (op_data->result_type) {
@@ -930,7 +919,6 @@ 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)
@@ -944,43 +932,13 @@ 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)
 {
-       CancelData *cd = user_data;
-       AsyncOpData *op_data;
-       GHashTable *pending_ops;
+       AsyncOpData *op_data = user_data;
        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;
@@ -1003,21 +961,14 @@ 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;
         * also schedule with priority higher than gtk+ uses
         * for animations (check docs for G_PRIORITY_HIGH_IDLE) */
-       op_data->cancel_idle_id = g_idle_add_full (G_PRIORITY_DEFAULT, e_gdbus_op_cancelled_idle_cb, cd, 
cancel_data_free);
+       op_data->cancel_idle_id = g_idle_add_full (G_PRIORITY_DEFAULT, e_gdbus_op_cancelled_idle_cb, NULL, 
NULL);
 }
 
 static void
@@ -1421,21 +1372,9 @@ e_gdbus_proxy_sync_ready_cb (GObject *proxy,
                              GAsyncResult *result,
                              gpointer 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;
-       }
+       SyncOpData *sync_data = user_data;
 
+       g_return_if_fail (sync_data != NULL);
        g_return_if_fail (sync_data->flag != NULL);
 
        switch (sync_data->out_type) {
@@ -1478,17 +1417,12 @@ 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;
@@ -1511,7 +1445,6 @@ 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;
        }
 
@@ -1519,37 +1452,30 @@ 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, GINT_TO_POINTER (sync_opid));
+               start (proxy, cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
        } break;
        case E_GDBUS_TYPE_BOOLEAN: {
                EGdbusCallStartBoolean start = start_func;
-               start (proxy, * ((gboolean *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, 
GINT_TO_POINTER (sync_opid));
+               start (proxy, * ((gboolean *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, 
&sync_data);
        } break;
        case E_GDBUS_TYPE_STRING: {
                EGdbusCallStartString start = start_func;
-               start (proxy, (const gchar *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, 
GINT_TO_POINTER (sync_opid));
+               start (proxy, (const gchar *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
        } break;
        case E_GDBUS_TYPE_STRV: {
                EGdbusCallStartStrv start = start_func;
-               start (proxy, (const gchar * const *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, 
GINT_TO_POINTER (sync_opid));
+               start (proxy, (const gchar * const *) in_value, cancellable, e_gdbus_proxy_sync_ready_cb, 
&sync_data);
        } break;
        case E_GDBUS_TYPE_UINT: {
                EGdbusCallStartUint start = start_func;
-               start (proxy, * ((guint *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, 
GINT_TO_POINTER (sync_opid));
+               start (proxy, * ((guint *) in_value), cancellable, e_gdbus_proxy_sync_ready_cb, &sync_data);
        } 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;
        }
 
@@ -1578,12 +1504,8 @@ 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;
 }