GNOME.org
 

[evolution-data-server] Rationalise fragmented locking. Use a recursive lock everywhere in case of problems. Unify get_delta

commit c59761e781ae3703d74649cfb2b7e9946946daf0
Author: Michael Meeks <michael meeks novell com>
Date:   Thu Mar 4 17:10:59 2010 +0000

    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 files 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;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]