[evolution-data-server/openismus-work-master: 2/2] Avoid race conditions when concurrently writing contacts in the addressbook



commit 18c46dd841bc46649c6ffe87e5c0513ef28fdcbf
Author: Tristan Van Berkom <tristanvb openismus com>
Date:   Mon Nov 12 16:25:49 2012 +0900

    Avoid race conditions when concurrently writing contacts in the addressbook
    
    This patch checks the incomming revision against the existing revision
    for the local file backend and rejects the commit with a newly added
    error code E_DATA_BOOK_STATUS_BAD_REVISION, which can be checked by the
    client in order to retry the commit.
    
    Note also, the revision format has changed to include the backend revision
    counter which ensures revisions are unique (even when multiple revisions
    occur inside the same second).

 addressbook/backends/file/e-book-backend-file.c |   83 ++++++++++++++++++-----
 addressbook/libebook/e-book-types.h             |    3 +-
 2 files changed, 67 insertions(+), 19 deletions(-)
---
diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c
index 6fb1483..510a300 100644
--- a/addressbook/backends/file/e-book-backend-file.c
+++ b/addressbook/backends/file/e-book-backend-file.c
@@ -69,6 +69,7 @@ struct _EBookBackendFilePrivate {
 	gchar     *photo_dirname;
 	gchar     *revision;
 	gint       rev_counter;
+	GRWLock    lock;
 
 	EBookBackendSqliteDB *sqlitedb;
 };
@@ -612,7 +613,7 @@ e_book_backend_file_new_revision (EBookBackendFile *bf)
  * every DB modification is going to really be an overhead.
  */
 static void
-e_book_backend_file_bump_revision (EBookBackendFile *bf)
+e_book_backend_file_bump_revision_locked (EBookBackendFile *bf)
 {
 	GError *error = NULL;
 
@@ -635,6 +636,14 @@ e_book_backend_file_bump_revision (EBookBackendFile *bf)
 }
 
 static void
+e_book_backend_file_bump_revision (EBookBackendFile *bf)
+{
+	g_rw_lock_writer_lock (&(bf->priv->lock));
+	e_book_backend_file_bump_revision_locked (bf);
+	g_rw_lock_writer_unlock (&(bf->priv->lock));
+}
+
+static void
 e_book_backend_file_load_revision (EBookBackendFile *bf)
 {
 	GError *error = NULL;
@@ -653,18 +662,14 @@ e_book_backend_file_load_revision (EBookBackendFile *bf)
 }
 
 static void
-set_revision (EContact *contact)
+set_revision (EBookBackendFile *bf,
+	      EContact *contact)
 {
-	gchar time_string[100] = {0};
-	const struct tm *tm = NULL;
-	time_t t;
-
-	t = time (NULL);
-	tm = gmtime (&t);
-	if (tm)
-		strftime (time_string, 100, "%Y-%m-%dT%H:%M:%SZ", tm);
-	e_contact_set (contact, E_CONTACT_REV, time_string);
+	gchar *rev;
 
+	rev = e_book_backend_file_new_revision (bf);
+	e_contact_set (contact, E_CONTACT_REV, rev);
+	g_free (rev);
 }
 
 /****************************************************************
@@ -709,7 +714,7 @@ do_create (EBookBackendFile *bf,
 
 		rev = e_contact_get_const (contact, E_CONTACT_REV);
 		if (!(rev && *rev))
-			set_revision (contact);
+			set_revision (bf, contact);
 
 		status = maybe_transform_vcard_for_photo (bf, NULL, contact, perror);
 
@@ -769,9 +774,13 @@ e_book_backend_file_create_contacts (EBookBackendSync *backend,
 {
 	EBookBackendFile *bf = E_BOOK_BACKEND_FILE (backend);
 
+	g_rw_lock_writer_lock (&(bf->priv->lock));
+
 	if (do_create (bf, vcards, added_contacts, perror)) {
-		e_book_backend_file_bump_revision (bf);
+		e_book_backend_file_bump_revision_locked (bf);
 	}
+
+	g_rw_lock_writer_unlock (&(bf->priv->lock));
 }
 
 static void
@@ -788,6 +797,8 @@ e_book_backend_file_remove_contacts (EBookBackendSync *backend,
 	gboolean          delete_failed = FALSE;
 	const GSList     *l;
 
+	g_rw_lock_writer_lock (&(bf->priv->lock));
+
 	for (l = id_list; l != NULL; l = l->next) {
 		const gchar *id;
 		EContact *contact;
@@ -834,13 +845,16 @@ e_book_backend_file_remove_contacts (EBookBackendSync *backend,
 			g_propagate_error (perror, local_error);
 		}
 
+		e_book_backend_file_bump_revision_locked (bf);
+
 		*ids = removed_ids;
 	} else {
 		*ids = NULL;
 		e_util_free_string_slist (removed_ids);
 	}
 
-	e_book_backend_file_bump_revision (bf);
+	g_rw_lock_writer_unlock (&(bf->priv->lock));
+
 	e_util_free_object_slist (removed_contacts);
 }
 
@@ -864,9 +878,12 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 		return;
 	}
 
+	g_rw_lock_writer_lock (&(bf->priv->lock));
+
 	for (l = vcards; l != NULL; l = l->next) {
 		gchar *id;
 		EContact *contact, *old_contact;
+		const gchar *contact_rev, *old_contact_rev;
 
 		contact = e_contact_new_from_vcard (l->data);
 		id = e_contact_get (contact, E_CONTACT_UID);
@@ -893,7 +910,21 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 			g_object_unref (contact);
 			break;
 		}
-		old_contacts = g_slist_prepend (old_contacts, old_contact);
+
+		contact_rev = e_contact_get_const (contact, E_CONTACT_REV);
+		old_contact_rev = e_contact_get_const (old_contact, E_CONTACT_REV);
+
+		if (!contact_rev || !old_contact_rev ||
+		    strcmp (contact_rev, old_contact_rev) != 0) {
+			g_propagate_error (perror, EDB_ERROR_EX (BAD_REVISION, _("Out of sync revision")));
+
+			status = STATUS_ERROR;
+
+			g_free (id);
+			g_object_unref (contact);
+			g_object_unref (old_contact);
+			break;
+		}
 
 		/* Transform incomming photo blobs to uris before storing this to the DB */
 		status = maybe_transform_vcard_for_photo (bf, old_contact, contact, &local_error);
@@ -908,8 +939,9 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 		}
 
 		/* update the revision (modified time of contact) */
-		set_revision (contact);
+		set_revision (bf, contact);
 
+		old_contacts      = g_slist_prepend (old_contacts, old_contact);
 		modified_contacts = g_slist_prepend (modified_contacts, contact);
 		ids               = g_slist_prepend (ids, id);
 	}
@@ -937,6 +969,11 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 		}
 	}
 
+	if (status != STATUS_ERROR)
+		e_book_backend_file_bump_revision_locked (bf);
+
+	g_rw_lock_writer_unlock (&(bf->priv->lock));
+
 	if (status != STATUS_ERROR) {
 		*contacts = g_slist_reverse (modified_contacts);
 	} else {
@@ -946,8 +983,6 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 
 	e_util_free_string_slist (ids);
 	g_slist_free_full (old_contacts, g_object_unref);
-
-	e_book_backend_file_bump_revision (bf);
 }
 
 static void
@@ -965,9 +1000,12 @@ e_book_backend_file_get_contact (EBookBackendSync *backend,
 		return;
 	}
 
+	g_rw_lock_reader_lock (&(bf->priv->lock));
+
 	*vcard = e_book_backend_sqlitedb_get_vcard_string (bf->priv->sqlitedb,
 							   SQLITEDB_FOLDER_ID, id,
 							   NULL, NULL, perror);
+	g_rw_lock_reader_unlock (&(bf->priv->lock));
 }
 
 static void
@@ -990,10 +1028,12 @@ e_book_backend_file_get_contact_list (EBookBackendSync *backend,
 		return;
 	}
 
+	g_rw_lock_reader_lock (&(bf->priv->lock));
 	summary_list = e_book_backend_sqlitedb_search (
 		bf->priv->sqlitedb, SQLITEDB_FOLDER_ID,
 		query, NULL,
 		NULL, NULL, &local_error);
+	g_rw_lock_reader_unlock (&(bf->priv->lock));
 
 	if (summary_list) {
 
@@ -1035,10 +1075,12 @@ e_book_backend_file_get_contact_list_uids (EBookBackendSync *backend,
 		return;
 	}
 
+	g_rw_lock_reader_lock (&(bf->priv->lock));
 	uids = e_book_backend_sqlitedb_search_uids (
 		bf->priv->sqlitedb,
 		SQLITEDB_FOLDER_ID,
 		query, NULL, &local_error);
+	g_rw_lock_reader_unlock (&(bf->priv->lock));
 
 	if (uids == NULL && local_error != NULL) {
 		g_warning ("Failed to fetch contact ids: %s", local_error->message);
@@ -1150,11 +1192,13 @@ book_view_thread (gpointer data)
 	d (printf ("signalling parent thread\n"));
 	e_flag_set (closure->running);
 
+	g_rw_lock_reader_lock (&(bf->priv->lock));
 	summary_list = e_book_backend_sqlitedb_search (
 		bf->priv->sqlitedb,
 		SQLITEDB_FOLDER_ID,
 		query, fields_of_interest,
 		NULL, NULL, &local_error);
+	g_rw_lock_reader_unlock (&(bf->priv->lock));
 
 	if (!summary_list && local_error != NULL) {
 		g_warning (G_STRLOC ": Failed to query initial contacts: %s", local_error->message);
@@ -1564,6 +1608,7 @@ e_book_backend_file_finalize (GObject *object)
 
 	g_free (priv->photo_dirname);
 	g_free (priv->revision);
+	g_rw_lock_clear (&(priv->lock));
 
 	/* Chain up to parent's finalize() method. */
 	G_OBJECT_CLASS (e_book_backend_file_parent_class)->finalize (object);
@@ -1605,6 +1650,8 @@ e_book_backend_file_init (EBookBackendFile *backend)
 {
 	backend->priv = E_BOOK_BACKEND_FILE_GET_PRIVATE (backend);
 
+	g_rw_lock_init (&(backend->priv->lock));
+
 	g_signal_connect (
 		backend, "notify::online",
 		G_CALLBACK (e_book_backend_file_notify_online_cb), NULL);
diff --git a/addressbook/libebook/e-book-types.h b/addressbook/libebook/e-book-types.h
index d778d91..14f9bd6 100644
--- a/addressbook/libebook/e-book-types.h
+++ b/addressbook/libebook/e-book-types.h
@@ -84,7 +84,8 @@ typedef enum {
 	E_DATA_BOOK_STATUS_NO_SPACE,
 	E_DATA_BOOK_STATUS_INVALID_ARG,
 	E_DATA_BOOK_STATUS_NOT_SUPPORTED,
-	E_DATA_BOOK_STATUS_NOT_OPENED
+	E_DATA_BOOK_STATUS_NOT_OPENED,
+	E_DATA_BOOK_STATUS_BAD_REVISION
 } EDataBookStatus;
 
 #ifndef EDS_DISABLE_DEPRECATED



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]