From c59761e781ae3703d74649cfb2b7e9946946daf0 Mon Sep 17 00:00:00 2001 From: Michael Meeks Date: Thu, 4 Mar 2010 17:10:59 +0000 Subject: [PATCH] Rationalise fragmented locking. Use a recursive lock everywhere in case of problems. Unify get_deltas lock with the main connection lock, yet drop it for the high latency get_objects call. Fix memory corruption issue bgo#611815 --- .../backends/groupwise/e-cal-backend-groupwise.c | 129 +++++++++++---------- 1 file changed, 67 insertions(+), 62 deletions(-) diff --git a/calendar/backends/groupwise/e-cal-backend-groupwise.c b/calendar/backends/groupwise/e-cal-backend-groupwise.c index af50306..6b3169d 100644 --- a/calendar/backends/groupwise/e-cal-backend-groupwise.c +++ b/calendar/backends/groupwise/e-cal-backend-groupwise.c @@ -72,7 +72,7 @@ typedef struct { /* Private part of the CalBackendGroupwise structure */ struct _ECalBackendGroupwisePrivate { /* A mutex to control access to the private structure */ - GMutex *mutex; + GStaticRecMutex rec_mutex; EGwConnection *cnc; ECalBackendStore *store; gboolean read_only; @@ -101,6 +101,9 @@ struct _ECalBackendGroupwisePrivate { SyncDelta *dlock; }; +#define PRIV_LOCK(p) (g_static_rec_mutex_lock (&(p)->rec_mutex)) +#define PRIV_UNLOCK(p) (g_static_rec_mutex_unlock (&(p)->rec_mutex)) + static void e_cal_backend_groupwise_dispose (GObject *object); static void e_cal_backend_groupwise_finalize (GObject *object); static void sanitize_component (ECalBackendSync *backend, ECalComponent *comp, gchar *server_uid); @@ -152,8 +155,6 @@ e_cal_backend_groupwise_get_default_zone (ECalBackendGroupwise *cbgw) { return cbgw->priv->default_zone; } -static GMutex *mutex = NULL; - static const gchar * get_element_type (icalcomponent_kind kind) { @@ -197,11 +198,7 @@ populate_cache (ECalBackendGroupwise *cbgw) kind = e_cal_backend_get_kind (E_CAL_BACKEND (cbgw)); total = priv->total_count; - if (!mutex) { - mutex = g_mutex_new (); - } - - g_mutex_lock (mutex); + PRIV_LOCK (priv); type = get_element_type (kind); @@ -239,7 +236,7 @@ populate_cache (ECalBackendGroupwise *cbgw) "recipients message recipientStatus attachments default peek", filter[i], &cursor); if (status != E_GW_CONNECTION_STATUS_OK) { e_cal_backend_groupwise_notify_error_code (cbgw, status); - g_mutex_unlock (mutex); + PRIV_UNLOCK (priv); return status; } done = FALSE; @@ -259,7 +256,7 @@ populate_cache (ECalBackendGroupwise *cbgw) status = e_gw_connection_read_cursor (priv->cnc, priv->container_id, cursor, forward, CURSOR_ITEM_LIMIT, position, &list); if (status != E_GW_CONNECTION_STATUS_OK) { e_cal_backend_groupwise_notify_error_code (cbgw, status); - g_mutex_unlock (mutex); + PRIV_UNLOCK (priv); return status; } for (l = list; l != NULL; l = g_list_next(l)) { @@ -307,7 +304,7 @@ populate_cache (ECalBackendGroupwise *cbgw) } e_cal_backend_notify_view_progress (E_CAL_BACKEND (cbgw), "", 100); - g_mutex_unlock (mutex); + PRIV_UNLOCK (priv); return E_GW_CONNECTION_STATUS_OK; } @@ -331,6 +328,8 @@ compare_ids (gconstpointer a, gconstpointer b) return 1; } +#define ATTEMPTS_KEY "attempts" + static gboolean get_deltas (gpointer handle) { @@ -345,10 +344,9 @@ get_deltas (gpointer handle) GPtrArray *uid_array = NULL; gchar *time_string = NULL; gchar t_str [26]; + gchar *attempts; + gchar *container_id; const gchar *serv_time; - static GStaticMutex connecting = G_STATIC_MUTEX_INIT; - const gchar *key = "attempts"; - const gchar *attempts; const gchar *position; EGwFilter *filter; @@ -362,19 +360,21 @@ get_deltas (gpointer handle) if (!handle) return FALSE; + cbgw = (ECalBackendGroupwise *) handle; - priv= cbgw->priv; + priv = cbgw->priv; + if (priv->mode == CAL_MODE_LOCAL) + return FALSE; + + PRIV_LOCK (priv); + kind = e_cal_backend_get_kind (E_CAL_BACKEND (cbgw)); cnc = priv->cnc; + store = priv->store; item_list = NULL; - if (priv->mode == CAL_MODE_LOCAL) - return FALSE; - - attempts = e_cal_backend_store_get_key_value (store, key); - - g_static_mutex_lock (&connecting); + attempts = g_strdup (e_cal_backend_store_get_key_value (store, ATTEMPTS_KEY)); serv_time = e_cal_backend_store_get_key_value (store, SERVER_UTC_TIME); if (serv_time) { @@ -405,31 +405,39 @@ get_deltas (gpointer handle) e_gw_filter_add_filter_component (filter, E_GW_FILTER_OP_EQUAL, "@type", get_element_type (kind)); e_gw_filter_group_conditions (filter, E_GW_FILTER_OP_AND, 2); - status = e_gw_connection_get_items (cnc, cbgw->priv->container_id, "attachments recipients message recipientStatus default peek", filter, &item_list); + container_id = g_strdup (cbgw->priv->container_id); + PRIV_UNLOCK (priv); + + status = e_gw_connection_get_items (cnc, container_id, "attachments recipients message recipientStatus default peek", filter, &item_list); if (status == E_GW_CONNECTION_STATUS_INVALID_CONNECTION) - status = e_gw_connection_get_items (cnc, cbgw->priv->container_id, "attachments recipients message recipientStatus default peek", filter, &item_list); + status = e_gw_connection_get_items (cnc, container_id, "attachments recipients message recipientStatus default peek", filter, &item_list); + PRIV_LOCK (priv); + + g_free (container_id); g_object_unref (filter); if (status != E_GW_CONNECTION_STATUS_OK) { const gchar *msg = NULL; + gint failures; - if (!attempts) { - e_cal_backend_store_put_key_value (store, key, "2"); - } else { - gint failures; - failures = g_ascii_strtod(attempts, NULL) + 1; - e_cal_backend_store_put_key_value (store, key, GINT_TO_POINTER (failures)); - } + if (!attempts) + failures = 2; + else + failures = g_ascii_strtod (attempts, NULL) + 1; + g_free (attempts); + attempts = g_strdup_printf ("%d", failures); + e_cal_backend_store_put_key_value (store, ATTEMPTS_KEY, attempts); + g_free (attempts); if (status == E_GW_CONNECTION_STATUS_NO_RESPONSE) { - g_static_mutex_unlock (&connecting); + PRIV_UNLOCK (priv); return TRUE; } msg = e_gw_connection_get_error_message (status); - g_static_mutex_unlock (&connecting); + PRIV_UNLOCK (priv); return TRUE; } @@ -501,10 +509,11 @@ get_deltas (gpointer handle) if (attempts) { tm.tm_min += (time_interval * g_ascii_strtod (attempts, NULL)); - e_cal_backend_store_put_key_value (store, key, NULL); + e_cal_backend_store_put_key_value (store, ATTEMPTS_KEY, NULL); } else { tm.tm_min += time_interval; } + g_free (attempts); strftime (t_str, 26, "%Y-%m-%dT%H:%M:%SZ", &tm); time_string = g_strdup (t_str); @@ -530,12 +539,12 @@ get_deltas (gpointer handle) if (status != E_GW_CONNECTION_STATUS_OK) { if (status == E_GW_CONNECTION_STATUS_NO_RESPONSE) { - g_static_mutex_unlock (&connecting); + PRIV_UNLOCK (priv); return TRUE; } e_cal_backend_groupwise_notify_error_code (cbgw, status); - g_static_mutex_unlock (&connecting); + PRIV_UNLOCK (priv); return TRUE; } @@ -679,7 +688,7 @@ get_deltas (gpointer handle) g_slist_free (cache_ids); } - g_static_mutex_unlock (&connecting); + PRIV_UNLOCK (priv); return TRUE; } @@ -791,9 +800,9 @@ e_cal_backend_groupwise_refresh_calendar (ECalBackendGroupwise *cbgw) if (priv->mode == CAL_MODE_LOCAL) return; - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); delta_started = fetch_deltas (cbgw); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); /* Emit the signal if the delta is already running */ if (!delta_started) @@ -892,9 +901,9 @@ cache_init (ECalBackendGroupwise *cbgw) } } - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); fetch_deltas (cbgw); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return NULL; } @@ -1077,7 +1086,6 @@ connect_to_server (ECalBackendGroupwise *cbgw) e_cal_backend_cache_remove (uri, source_type); priv->store = (ECalBackendStore *) e_cal_backend_file_store_new (uri, source_type); if (!priv->store) { - g_mutex_unlock (priv->mutex); e_cal_backend_notify_error (E_CAL_BACKEND (cbgw), _("Could not create cache file")); return GNOME_Evolution_Calendar_OtherError; } @@ -1156,10 +1164,7 @@ e_cal_backend_groupwise_finalize (GObject *object) priv->dthread = NULL; } - if (priv->mutex) { - g_mutex_free (priv->mutex); - priv->mutex = NULL; - } + g_static_rec_mutex_free (&priv->rec_mutex); if (priv->cnc) { g_object_unref (priv->cnc); @@ -1337,7 +1342,7 @@ e_cal_backend_groupwise_open (ECalBackendSync *backend, EDataCal *cal, gboolean cbgw = E_CAL_BACKEND_GROUPWISE (backend); priv = cbgw->priv; - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); cbgw->priv->read_only = FALSE; @@ -1367,7 +1372,7 @@ e_cal_backend_groupwise_open (ECalBackendSync *backend, EDataCal *cal, gboolean display_contents = e_source_get_property (esource, "offline_sync"); if (!display_contents || !g_str_equal (display_contents, "1")) { - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return GNOME_Evolution_Calendar_RepositoryOffline; } @@ -1378,7 +1383,7 @@ e_cal_backend_groupwise_open (ECalBackendSync *backend, EDataCal *cal, gboolean e_cal_backend_cache_remove (uri, source_type); priv->store = (ECalBackendStore *) e_cal_backend_file_store_new (uri, source_type); if (!priv->store) { - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); e_cal_backend_notify_error (E_CAL_BACKEND (cbgw), _("Could not create cache file")); return GNOME_Evolution_Calendar_OtherError; @@ -1387,7 +1392,7 @@ e_cal_backend_groupwise_open (ECalBackendSync *backend, EDataCal *cal, gboolean e_cal_backend_store_set_default_timezone (priv->store, priv->default_zone); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return GNOME_Evolution_Calendar_Success; } @@ -1420,7 +1425,7 @@ e_cal_backend_groupwise_open (ECalBackendSync *backend, EDataCal *cal, gboolean /* FIXME: no need to set it online here when we implement the online/offline stuff correctly */ status = connect_to_server (cbgw); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return status; } @@ -1433,13 +1438,13 @@ e_cal_backend_groupwise_remove (ECalBackendSync *backend, EDataCal *cal) cbgw = E_CAL_BACKEND_GROUPWISE (backend); priv = cbgw->priv; - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); /* remove the cache */ if (priv->store) e_cal_backend_store_remove (priv->store); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return GNOME_Evolution_Calendar_Success; } @@ -1486,7 +1491,7 @@ e_cal_backend_groupwise_set_mode (ECalBackend *backend, CalMode mode) return; } - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); priv->mode_changed = TRUE; switch (mode) { @@ -1514,7 +1519,7 @@ e_cal_backend_groupwise_set_mode (ECalBackend *backend, CalMode mode) cal_mode_to_corba (mode)); } - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); } static ECalBackendSyncStatus @@ -1555,12 +1560,12 @@ e_cal_backend_groupwise_get_object (ECalBackendSync *backend, EDataCal *cal, con priv = cbgw->priv; - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); /* search the object in the cache */ comp = e_cal_backend_store_get_component (priv->store, uid, rid); if (comp) { - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); if (e_cal_backend_get_kind (E_CAL_BACKEND (backend)) == icalcomponent_isa (e_cal_component_get_icalcomponent (comp))) *object = e_cal_component_get_as_string (comp); @@ -1572,7 +1577,7 @@ e_cal_backend_groupwise_get_object (ECalBackendSync *backend, EDataCal *cal, con return *object ? GNOME_Evolution_Calendar_Success : GNOME_Evolution_Calendar_ObjectNotFound; } - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); /* callers will never have a uuid that is in server but not in cache */ return GNOME_Evolution_Calendar_ObjectNotFound; @@ -1663,7 +1668,7 @@ e_cal_backend_groupwise_get_object_list (ECalBackendSync *backend, EDataCal *cal cbgw = E_CAL_BACKEND_GROUPWISE (backend); priv = cbgw->priv; - g_mutex_lock (priv->mutex); + PRIV_LOCK (priv); g_message (G_STRLOC ": Getting object list (%s)", sexp); @@ -1672,7 +1677,7 @@ e_cal_backend_groupwise_get_object_list (ECalBackendSync *backend, EDataCal *cal cbsexp = e_cal_backend_sexp_new (sexp); if (!cbsexp) { - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return GNOME_Evolution_Calendar_InvalidQuery; } @@ -1694,7 +1699,7 @@ e_cal_backend_groupwise_get_object_list (ECalBackendSync *backend, EDataCal *cal g_slist_foreach (components, (GFunc) g_object_unref, NULL); g_slist_free (components); - g_mutex_unlock (priv->mutex); + PRIV_UNLOCK (priv); return GNOME_Evolution_Calendar_Success; } @@ -1889,7 +1894,7 @@ e_cal_backend_groupwise_internal_get_default_timezone (ECalBackend *backend) static icaltimezone * e_cal_backend_groupwise_internal_get_timezone (ECalBackend *backend, const gchar *tzid) { - icaltimezone *zone; + icaltimezone *zone = NULL; ECalBackendGroupwise *cbgw; cbgw = E_CAL_BACKEND_GROUPWISE (backend); @@ -2813,7 +2818,7 @@ e_cal_backend_groupwise_init (ECalBackendGroupwise *cbgw) priv->sendoptions_sync_timeout = 0; /* create the mutex for thread safety */ - priv->mutex = g_mutex_new (); + g_static_rec_mutex_init (&priv->rec_mutex); cbgw->priv = priv; -- 2.7.4