From 779f1ea93d54543548a49eb1649ea1f86fb0734f Mon Sep 17 00:00:00 2001 From: Jeremy Whiting Date: Fri, 20 Jul 2012 14:57:56 -0600 Subject: [PATCH] Removed locks in trf-persona-store that are not needed. Minor style fixes based on review of previous changes. --- backends/libsocialweb/lib/swf-persona-store.vala | 16 ++++-- backends/libsocialweb/sw-backend.vala | 66 +++++++++++++----------- backends/telepathy/lib/tpf-persona-store.vala | 1 - backends/telepathy/tp-backend.vala | 2 + backends/tracker/lib/trf-persona-store.vala | 51 +++++++----------- tests/folks/async-locking.vala | 3 ++ 6 files changed, 72 insertions(+), 67 deletions(-) diff --git a/backends/libsocialweb/lib/swf-persona-store.vala b/backends/libsocialweb/lib/swf-persona-store.vala index 9ee7535..18e25f1 100644 --- a/backends/libsocialweb/lib/swf-persona-store.vala +++ b/backends/libsocialweb/lib/swf-persona-store.vala @@ -318,13 +318,18 @@ public class Swf.PersonaStore : Folks.PersonaStore { Internal.profiling_start ("preparing Swf.PersonaStore (ID: %s)", this.id); - if (!this._is_prepared && !this._prepare_pending) + if (this._is_prepared || this._prepare_pending) + { + return; + } + + try { this._prepare_pending = true; - + /* Get the service's capabilities. */ string[]? caps = null; - + try { caps = yield this._get_static_capabilities (); @@ -392,7 +397,6 @@ public class Swf.PersonaStore : Folks.PersonaStore this._contact_view = contact_view; this._is_prepared = true; - this._prepare_pending = false; this.notify_property ("is-prepared"); /* FIXME: for lsw Stores with 0 contacts or badly configured (or @@ -411,6 +415,10 @@ public class Swf.PersonaStore : Folks.PersonaStore this._contact_view.start (); } + finally + { + this._prepare_pending = false; + } Internal.profiling_end ("preparing Swf.PersonaStore (ID: %s)", this.id); } diff --git a/backends/libsocialweb/sw-backend.vala b/backends/libsocialweb/sw-backend.vala index db7d7de..6aa80b9 100644 --- a/backends/libsocialweb/sw-backend.vala +++ b/backends/libsocialweb/sw-backend.vala @@ -102,18 +102,18 @@ public class Folks.Backends.Sw.Backend : Folks.Backend return; } - try - { - this._prepare_pending = true; + /* Hold a ref. on the Backend while we wait for the callback from + * this._client.get_services() to prevent the Backend being + * destroyed in the mean time. See: bgo#665039. */ + this.ref (); - /* Hold a ref. on the Backend while we wait for the callback from - * this._client.get_services() to prevent the Backend being - * destroyed in the mean time. See: bgo#665039. */ - this.ref (); - - this._client = new Client (); - this._client.get_services((client, services) => + this._client = new Client (); + this._client.get_services((client, services) => + { + try { + this._prepare_pending = true; + foreach (var service_name in services) this.add_service (service_name); @@ -124,12 +124,12 @@ public class Folks.Backends.Sw.Backend : Folks.Backend this.notify_property ("is-quiescent"); this.unref (); - }); - } - finally - { - this._prepare_pending = false; - } + } + finally + { + this._prepare_pending = false; + } + }); Internal.profiling_end ("preparing Sw.Backend"); } @@ -144,25 +144,31 @@ public class Folks.Backends.Sw.Backend : Folks.Backend return; } - this._prepare_pending = true; - - foreach (var store in this._persona_stores.values) + try { - store.removed.disconnect (this.store_removed_cb); - this.persona_store_removed (store); - } + this._prepare_pending = true; + + foreach (var store in this._persona_stores.values) + { + store.removed.disconnect (this.store_removed_cb); + this.persona_store_removed (store); + } - this._client = null; + this._client = null; - this._persona_stores.clear (); - this.notify_property ("persona-stores"); + this._persona_stores.clear (); + this.notify_property ("persona-stores"); - this._is_quiescent = false; - this.notify_property ("is-quiescent"); + this._is_quiescent = false; + this.notify_property ("is-quiescent"); - this._is_prepared = false; - this._prepare_pending = false; - this.notify_property ("is-prepared"); + this._is_prepared = false; + this.notify_property ("is-prepared"); + } + finally + { + this._prepare_pending = false; + } } private void add_service (string service_name) diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala index fc6486d..0091547 100644 --- a/backends/telepathy/lib/tpf-persona-store.vala +++ b/backends/telepathy/lib/tpf-persona-store.vala @@ -563,7 +563,6 @@ public class Tpf.PersonaStore : Folks.PersonaStore "(ID: %s)", this.id); this._is_prepared = true; - this._prepare_pending = false; this.notify_property ("is-prepared"); } finally diff --git a/backends/telepathy/tp-backend.vala b/backends/telepathy/tp-backend.vala index d6acbae..0734dbb 100644 --- a/backends/telepathy/tp-backend.vala +++ b/backends/telepathy/tp-backend.vala @@ -137,6 +137,8 @@ public class Folks.Backends.Tp.Backend : Folks.Backend try { + this._prepare_pending = true; + this._account_manager.account_enabled.disconnect ( this._account_enabled_cb); this._account_manager.account_validity_changed.disconnect ( diff --git a/backends/tracker/lib/trf-persona-store.vala b/backends/tracker/lib/trf-persona-store.vala index 437d033..a95d8ab 100644 --- a/backends/tracker/lib/trf-persona-store.vala +++ b/backends/tracker/lib/trf-persona-store.vala @@ -1142,10 +1142,6 @@ public class Trf.PersonaStore : Folks.PersonaStore this.removed (); throw new PersonaStoreError.INVALID_ARGUMENT (e3.message); } - finally - { - this._prepare_pending = false; - } } finally { @@ -1326,14 +1322,11 @@ public class Trf.PersonaStore : Folks.PersonaStore if (e.pred_id == rdf_type_id && e.object_id == nco_person_id) { - lock (this._personas) + var removed_p = this._personas.get (p_id); + if (removed_p != null) { - var removed_p = this._personas.get (p_id); - if (removed_p != null) - { - removed_personas.add (removed_p); - _personas.unset (removed_p.iid); - } + removed_personas.add (removed_p); + _personas.unset (removed_p.iid); } } else @@ -1363,15 +1356,12 @@ public class Trf.PersonaStore : Folks.PersonaStore var subject_tracker_id = e.subject_id.to_string (); var p_id = Trf.Persona.build_iid (this.id, subject_tracker_id); Trf.Persona persona; - lock (this._personas) + persona = this._personas.get (p_id); + if (persona == null) { - persona = this._personas.get (p_id); - if (persona == null) - { - persona = new Trf.Persona (this, subject_tracker_id); - this._personas.set (persona.iid, persona); - added_personas.add (persona); - } + persona = new Trf.Persona (this, subject_tracker_id); + this._personas.set (persona.iid, persona); + added_personas.add (persona); } yield this._do_update (persona, e); } @@ -1389,21 +1379,18 @@ public class Trf.PersonaStore : Folks.PersonaStore try { Sparql.Cursor cursor = yield this._connection.query_async (query); - lock (this._personas) + while (cursor.next ()) { - while (cursor.next ()) + int tracker_id = + (int) cursor.get_integer (Trf.Fields.TRACKER_ID); + var p_id = + Trf.Persona.build_iid (this.id, tracker_id.to_string ()); + if (this._personas.get (p_id) == null) { - int tracker_id = - (int) cursor.get_integer (Trf.Fields.TRACKER_ID); - var p_id = - Trf.Persona.build_iid (this.id, tracker_id.to_string ()); - if (this._personas.get (p_id) == null) - { - var persona = new Trf.Persona (this, - tracker_id.to_string (), cursor); - this._personas.set (persona.iid, persona); - added_personas.add (persona); - } + var persona = new Trf.Persona (this, + tracker_id.to_string (), cursor); + this._personas.set (persona.iid, persona); + added_personas.add (persona); } } diff --git a/tests/folks/async-locking.vala b/tests/folks/async-locking.vala index 9957031..fd424fe 100644 --- a/tests/folks/async-locking.vala +++ b/tests/folks/async-locking.vala @@ -9,6 +9,9 @@ public class AsyncLockingTests : Folks.TestCase int _total_calls; MainLoop _loop; + // This test was added to ensure removing lock (this._is_prepared) in prepare + // Methods is not going to cause a problem. + // See bug: https://bugzilla.gnome.org/show_bug.cgi?id=652637 public AsyncLockingTests () { base ("AsyncLocking"); -- 2.7.4