[tracker/sparql-init: 2/2] libtracker-sparql: Fix deadlock on initialization
- From: Jürg Billeter <juergbi src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [tracker/sparql-init: 2/2] libtracker-sparql: Fix deadlock on initialization
- Date: Wed, 16 Mar 2011 12:08:16 +0000 (UTC)
commit d054e923c01dc7a43a58a705e56493ece47fcecf
Author: Jürg Billeter <j bitron ch>
Date: Wed Mar 16 13:02:34 2011 +0100
libtracker-sparql: Fix deadlock on initialization
Calling Connection.get_async followed by Connection.get in the same
thread led to a deadlock. This moves all initialization into a separate
thread when invoked asynchronously instead of just initializing the
database in a separate thread.
src/libtracker-data/libtracker-data.vapi | 1 -
src/libtracker-data/tracker-data-manager.c | 230 +++++--------------------
src/libtracker-data/tracker-data-manager.h | 13 --
src/libtracker-direct/tracker-direct.vala | 16 --
src/libtracker-sparql/tracker-backend.vala | 34 +----
src/libtracker-sparql/tracker-connection.vala | 65 ++++---
6 files changed, 81 insertions(+), 278 deletions(-)
---
diff --git a/src/libtracker-data/libtracker-data.vapi b/src/libtracker-data/libtracker-data.vapi
index 549b9f4..df73afe 100644
--- a/src/libtracker-data/libtracker-data.vapi
+++ b/src/libtracker-data/libtracker-data.vapi
@@ -198,7 +198,6 @@ namespace Tracker {
[CCode (cheader_filename = "libtracker-data/tracker-data-manager.h")]
namespace Data.Manager {
public bool init (DBManagerFlags flags, [CCode (array_length = false)] string[]? test_schema, out bool first_time, bool journal_check, uint select_cache_size, uint update_cache_size, BusyCallback? busy_callback, string? busy_status) throws DBInterfaceError;
- public async bool init_async (DBManagerFlags flags, [CCode (array_length = false)] string[]? test_schema, bool journal_check, uint select_cache_size, uint update_cache_size, BusyCallback? busy_callback, string? busy_status) throws DBInterfaceError;
public void shutdown ();
}
diff --git a/src/libtracker-data/tracker-data-manager.c b/src/libtracker-data/tracker-data-manager.c
index 3ce6805..f90a250 100644
--- a/src/libtracker-data/tracker-data-manager.c
+++ b/src/libtracker-data/tracker-data-manager.c
@@ -75,19 +75,6 @@ static gboolean initialized;
static gboolean in_journal_replay;
typedef struct {
- TrackerDBManagerFlags flags;
- gchar **test_schema;
- gboolean first_time; /* Unused atm */
- gboolean journal_check;
- guint select_cache_size;
- guint update_cache_size;
- TrackerBusyCallback busy_callback;
- gpointer busy_user_data;
- gchar *busy_operation;
- gboolean result;
-} InitAsyncData;
-
-typedef struct {
const gchar *from;
const gchar *to;
} Conversion;
@@ -3450,17 +3437,17 @@ load_ontologies_gvdb (GError **error)
g_free (filename);
}
-static gboolean
-data_manager_init_unlocked (TrackerDBManagerFlags flags,
- const gchar **test_schemas,
- gboolean *first_time,
- gboolean journal_check,
- guint select_cache_size,
- guint update_cache_size,
- TrackerBusyCallback busy_callback,
- gpointer busy_user_data,
- const gchar *busy_operation,
- GError **error)
+gboolean
+tracker_data_manager_init (TrackerDBManagerFlags flags,
+ const gchar **test_schemas,
+ gboolean *first_time,
+ gboolean journal_check,
+ guint select_cache_size,
+ guint update_cache_size,
+ TrackerBusyCallback busy_callback,
+ gpointer busy_user_data,
+ const gchar *busy_operation,
+ GError **error)
{
TrackerDBInterface *iface;
gboolean is_first_time_index, read_journal, check_ontology;
@@ -3857,16 +3844,16 @@ data_manager_init_unlocked (TrackerDBManagerFlags flags,
initialized = TRUE;
tracker_data_manager_shutdown ();
- return data_manager_init_unlocked (flags,
- test_schemas,
- first_time,
- journal_check,
- select_cache_size,
- update_cache_size,
- busy_callback,
- busy_user_data,
- busy_operation,
- error);
+ return tracker_data_manager_init (flags,
+ test_schemas,
+ first_time,
+ journal_check,
+ select_cache_size,
+ update_cache_size,
+ busy_callback,
+ busy_user_data,
+ busy_operation,
+ error);
}
if (ontology_error) {
@@ -3923,16 +3910,16 @@ data_manager_init_unlocked (TrackerDBManagerFlags flags,
initialized = TRUE;
tracker_data_manager_shutdown ();
- return data_manager_init_unlocked (flags,
- test_schemas,
- first_time,
- journal_check,
- select_cache_size,
- update_cache_size,
- busy_callback,
- busy_user_data,
- busy_operation,
- error);
+ return tracker_data_manager_init (flags,
+ test_schemas,
+ first_time,
+ journal_check,
+ select_cache_size,
+ update_cache_size,
+ busy_callback,
+ busy_user_data,
+ busy_operation,
+ error);
}
if (ontology_error) {
@@ -4016,16 +4003,16 @@ data_manager_init_unlocked (TrackerDBManagerFlags flags,
initialized = TRUE;
tracker_data_manager_shutdown ();
- return data_manager_init_unlocked (flags,
- test_schemas,
- first_time,
- journal_check,
- select_cache_size,
- update_cache_size,
- busy_callback,
- busy_user_data,
- busy_operation,
- error);
+ return tracker_data_manager_init (flags,
+ test_schemas,
+ first_time,
+ journal_check,
+ select_cache_size,
+ update_cache_size,
+ busy_callback,
+ busy_user_data,
+ busy_operation,
+ error);
}
if (ontology_error) {
@@ -4136,141 +4123,6 @@ data_manager_init_unlocked (TrackerDBManagerFlags flags,
return TRUE;
}
-gboolean
-tracker_data_manager_init (TrackerDBManagerFlags flags,
- const gchar **test_schemas,
- gboolean *first_time,
- gboolean journal_check,
- guint select_cache_size,
- guint update_cache_size,
- TrackerBusyCallback busy_callback,
- gpointer busy_user_data,
- const gchar *busy_operation,
- GError **error)
-{
- static GStaticMutex my_mutex = G_STATIC_MUTEX_INIT;
- gboolean ret;
-
- /* This lock actually only protects 'initialized', but the whole function
- * is involved in setting it (as it's getting called recursively) */
-
- g_static_mutex_lock (&my_mutex);
-
- ret = data_manager_init_unlocked (flags,
- test_schemas,
- first_time,
- journal_check,
- select_cache_size,
- update_cache_size,
- busy_callback,
- busy_user_data,
- busy_operation,
- error);
-
- g_static_mutex_unlock (&my_mutex);
-
- return ret;
-}
-
-
-static void
-tracker_data_manager_init_thread (GSimpleAsyncResult *result,
- GObject *object,
- GCancellable *cancellable)
-{
- InitAsyncData *data;
- gboolean result_b;
- GError *internal_error = NULL;
-
- data = g_simple_async_result_get_op_res_gpointer (result);
-
- result_b = tracker_data_manager_init (data->flags,
- (const gchar **) data->test_schema,
- &data->first_time, /* Unused atm */
- data->journal_check,
- data->select_cache_size,
- data->update_cache_size,
- data->busy_callback,
- data->busy_user_data,
- data->busy_operation,
- &internal_error);
-
- if (internal_error) {
- g_simple_async_result_set_from_error (result, internal_error);
- g_error_free (internal_error);
- }
-
- data->result = result_b;
-}
-
-static void
-init_async_data_free (gpointer data)
-{
- InitAsyncData *d = data;
-
- g_strfreev (d->test_schema);
- g_free (d->busy_operation);
-
- g_free (data);
-}
-
-void
-tracker_data_manager_init_async (TrackerDBManagerFlags flags,
- const gchar **test_schema,
- gboolean journal_check,
- guint select_cache_size,
- guint update_cache_size,
- TrackerBusyCallback busy_callback,
- gpointer busy_user_data,
- const gchar *busy_operation,
- GAsyncReadyCallback callback,
- gpointer user_data)
-{
- GSimpleAsyncResult *result;
- InitAsyncData *data = g_new0 (InitAsyncData, 1);
-
- data->flags = flags;
- data->test_schema = g_strdupv ((gchar **) test_schema);
- data->journal_check = journal_check;
- data->select_cache_size = select_cache_size;
- data->update_cache_size = update_cache_size;
- data->busy_callback = busy_callback;
- data->busy_user_data = busy_user_data;
- data->busy_operation = g_strdup (busy_operation);
-
- result = g_simple_async_result_new (NULL,
- callback,
- user_data,
- (gpointer) tracker_data_manager_init_async);
-
- g_simple_async_result_set_op_res_gpointer (result,
- data,
- init_async_data_free);
-
- g_simple_async_result_run_in_thread (result,
- tracker_data_manager_init_thread,
- 0,
- NULL);
-}
-
-gboolean
-tracker_data_manager_init_finish (GAsyncResult *result,
- GError **error)
-{
- InitAsyncData *data;
- GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT (result);
-
- g_return_val_if_fail (G_IS_ASYNC_RESULT (result), FALSE);
-
- if (g_simple_async_result_propagate_error (res, error)) {
- return FALSE;
- }
-
- data = g_simple_async_result_get_op_res_gpointer (res);
-
- return data->result;
-}
-
void
tracker_data_manager_shutdown (void)
{
diff --git a/src/libtracker-data/tracker-data-manager.h b/src/libtracker-data/tracker-data-manager.h
index b8b1e05..4b4b0a0 100644
--- a/src/libtracker-data/tracker-data-manager.h
+++ b/src/libtracker-data/tracker-data-manager.h
@@ -56,19 +56,6 @@ gboolean tracker_data_manager_init (TrackerDBManagerFlags fl
const gchar *busy_operation,
GError **error);
-void tracker_data_manager_init_async (TrackerDBManagerFlags flags,
- const gchar **test_schema,
- gboolean journal_check,
- guint select_cache_size,
- guint update_cache_size,
- TrackerBusyCallback busy_callback,
- gpointer busy_user_data,
- const gchar *busy_operation,
- GAsyncReadyCallback callback,
- gpointer user_data);
-gboolean tracker_data_manager_init_finish (GAsyncResult *result,
- GError **error);
-
void tracker_data_manager_shutdown (void);
gboolean tracker_data_manager_reload (TrackerBusyCallback busy_callback,
gpointer busy_user_data,
diff --git a/src/libtracker-direct/tracker-direct.vala b/src/libtracker-direct/tracker-direct.vala
index 4d11e40..d19a3f9 100644
--- a/src/libtracker-direct/tracker-direct.vala
+++ b/src/libtracker-direct/tracker-direct.vala
@@ -42,22 +42,6 @@ public class Tracker.Direct.Connection : Tracker.Sparql.Connection {
initialized = true;
}
- public async override void init_async () throws Sparql.Error, IOError, DBusError {
- uint select_cache_size = 100;
- string env_cache_size = Environment.get_variable ("TRACKER_SPARQL_CACHE_SIZE");
-
- if (env_cache_size != null) {
- select_cache_size = env_cache_size.to_int();
- }
-
- try {
- yield Data.Manager.init_async (DBManagerFlags.READONLY, null, false, select_cache_size, 0, null, null);
- } catch (DBInterfaceError e) {
- throw new Sparql.Error.INTERNAL (e.message);
- }
- initialized = true;
- }
-
~Connection () {
// Clean up connection
if (initialized) {
diff --git a/src/libtracker-sparql/tracker-backend.vala b/src/libtracker-sparql/tracker-backend.vala
index 758fbb5..6a20714 100644
--- a/src/libtracker-sparql/tracker-backend.vala
+++ b/src/libtracker-sparql/tracker-backend.vala
@@ -20,8 +20,6 @@
[DBus (name = "org.freedesktop.Tracker1.Status")]
interface Tracker.Backend.Status : DBusProxy {
public abstract void wait () throws DBusError;
- [DBus (name = "Wait")]
- public abstract async void wait_async () throws DBusError;
}
class Tracker.Sparql.Backend : Connection {
@@ -51,14 +49,14 @@ class Tracker.Sparql.Backend : Connection {
status.set_default_timeout (int.MAX);
// Makes sure the sevice is available
- debug ("Waiting for service to become available synchronously...");
+ debug ("Waiting for service to become available...");
status.wait ();
debug ("Service is ready");
try {
debug ("Constructing connection, direct_only=%s", direct_only ? "true" : "false");
if (load_plugins (direct_only)) {
- debug ("Waiting for backend to become available synchronously...");
+ debug ("Waiting for backend to become available...");
if (direct != null) {
direct.init ();
} else {
@@ -71,34 +69,6 @@ class Tracker.Sparql.Backend : Connection {
}
}
- public async override void init_async () throws Sparql.Error, IOError, DBusError {
- Tracker.Backend.Status status = Bus.get_proxy_sync (BusType.SESSION,
- TRACKER_DBUS_SERVICE,
- TRACKER_DBUS_OBJECT_STATUS,
- DBusProxyFlags.DO_NOT_LOAD_PROPERTIES | DBusProxyFlags.DO_NOT_CONNECT_SIGNALS);
- status.set_default_timeout (int.MAX);
-
- // Makes sure the sevice is available
- debug ("Waiting for service to become available asynchronously...");
- yield status.wait_async ();
- debug ("Service is ready");
-
- try {
- debug ("Constructing connection, direct_only=%s", direct_only ? "true" : "false");
- if (load_plugins (direct_only)) {
- debug ("Waiting for backend to become available asynchronously...");
- if (direct != null) {
- yield direct.init_async ();
- } else {
- yield bus.init_async ();
- }
- debug ("Backend is ready");
- }
- } catch (GLib.Error e) {
- throw new Sparql.Error.INTERNAL (e.message);
- }
- }
-
public override Cursor query (string sparql, Cancellable? cancellable = null) throws Sparql.Error, IOError, DBusError
requires (bus != null || direct != null) {
debug ("%s(): '%s'", Log.METHOD, sparql);
diff --git a/src/libtracker-sparql/tracker-connection.vala b/src/libtracker-sparql/tracker-connection.vala
index 64b6aec..1899b20 100644
--- a/src/libtracker-sparql/tracker-connection.vala
+++ b/src/libtracker-sparql/tracker-connection.vala
@@ -117,34 +117,49 @@ public abstract class Tracker.Sparql.Connection : Object {
}
private async static new Connection get_internal_async (bool is_direct_only = false, Cancellable? cancellable = null) throws Sparql.Error, IOError, DBusError {
- door.lock ();
+ // fast path: avoid extra thread if connection is already available
+ if (door.trylock ()) {
+ // assign to owned variable to ensure it doesn't get freed between unlock and return
+ var result = singleton;
- // assign to owned variable to ensure it doesn't get freed between unlock and return
- var result = singleton;
- if (result != null) {
- assert (direct_only == is_direct_only);
door.unlock ();
- return result;
- }
-
- log_init ();
-
- /* the True is to assert that direct only is required */
- result = new Backend (is_direct_only);
- yield result.init_async ();
- if (cancellable != null && cancellable.is_cancelled ()) {
- door.unlock ();
- throw new IOError.CANCELLED ("Operation was cancelled");
+ if (result != null) {
+ assert (direct_only == is_direct_only);
+ return result;
+ }
}
- direct_only = is_direct_only;
- singleton = result;
- result.add_weak_pointer ((void**) (&singleton));
-
- door.unlock ();
-
- return singleton;
+ // run in a separate thread
+ Sparql.Error sparql_error = null;
+ IOError io_error = null;
+ DBusError dbus_error = null;
+ Connection result = null;
+
+ g_io_scheduler_push_job (job => {
+ try {
+ result = get_internal (is_direct_only, cancellable);
+ } catch (IOError e_io) {
+ io_error = e_io;
+ } catch (Sparql.Error e_spql) {
+ sparql_error = e_spql;
+ } catch (DBusError e_dbus) {
+ dbus_error = e_dbus;
+ }
+ get_internal_async.callback ();
+ return false;
+ });
+ yield;
+
+ if (sparql_error != null) {
+ throw sparql_error;
+ } else if (io_error != null) {
+ throw io_error;
+ } else if (dbus_error != null) {
+ throw dbus_error;
+ } else {
+ return result;
+ }
}
/**
@@ -203,10 +218,6 @@ public abstract class Tracker.Sparql.Connection : Object {
* these functions, a mutex is used to protect the loading of backends
* against potential race conditions. For synchronous calls, this function
* will always block if a previous connection get method has been called.
- * For asynchronous calls, this <emphasis>may</emphasis> block if another
- * synchronous or asynchronous call has been previously dispatched and is
- * still pending. We don't expect this to be a normal programming model when
- * using this API.
*
* All backends will call the D-Bus tracker-store API Wait() to make sure
* the store and databases are in the right state before any user based
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
