[tracker/wip/carlosg/ontology-race: 2/2] libtracker-data: Protect ontology objects with a mutex

[Carlos Garnacho <carlosg src gnome org>]

commit 721c46273c48d0b5a7ce949fcbd0b5c65bd3456a
Author: Carlos Garnacho <carlosg gnome org>
Date:   Tue Nov 24 13:58:25 2020 +0100

    libtracker-data: Protect ontology objects with a mutex
    
    In the case of readonly connections, we use the ontology gvdb as a
    fast path to regenerate the ontology, and fetch the actual data on
    demand.
    
    However, we use the ontology bits from the query machinery, so
    fetching this data on the fly may collide with other threads
    performing queries that require the same ontology resources. This
    may cause from leaks (because two threads get the same data, but
    one is lost) to crashes (because one thread frees data that the
    other is accessing).
    
    Make all gvdb extraction at a single point for those objects, and
    protect the operation with a mutex. As we can do this once, optimize
    the case that data has been already extracted (not needing a mutex
    lock/unlock then). But obviously, also handle the case that 2 or more
    threads contend on gvdb synchronization.
    
    Fixes: https://gitlab.gnome.org/GNOME/tracker/-/issues/272

 src/libtracker-data/tracker-class.c     |  64 ++++++----
 src/libtracker-data/tracker-namespace.c |  32 ++++-
 src/libtracker-data/tracker-property.c  | 203 +++++++++++++++++---------------
 3 files changed, 178 insertions(+), 121 deletions(-)
---
diff --git a/src/libtracker-data/tracker-class.c b/src/libtracker-data/tracker-class.c
index 3c87cbabb..c3fcdd2cd 100644
--- a/src/libtracker-data/tracker-class.c
+++ b/src/libtracker-data/tracker-class.c
@@ -40,6 +40,7 @@ struct _TrackerClassPrivate {
        guint notify : 1;
        guint use_gvdb : 1;
 
+       GMutex mutex;
 
        GArray *super_classes;
        GArray *domain_indexes;
@@ -73,6 +74,7 @@ tracker_class_init (TrackerClass *service)
        priv->domain_indexes = g_array_new (TRUE, TRUE, sizeof (TrackerProperty *));
        priv->last_domain_indexes = NULL;
        priv->last_super_classes = NULL;
+       g_mutex_init (&priv->mutex);
 }
 
 static void
@@ -115,6 +117,46 @@ tracker_class_new (gboolean use_gvdb)
        return service;
 }
 
+static void
+tracker_class_maybe_sync_from_gvdb (TrackerClass *service)
+{
+       TrackerClassPrivate *priv;
+       TrackerClass *super_class;
+       GVariant *variant;
+
+       priv = tracker_class_get_instance_private (service);
+
+       if (!priv->use_gvdb)
+               return;
+
+       g_mutex_lock (&priv->mutex);
+
+       /* In case the lock was contended, make the second lose */
+       if (!priv->use_gvdb)
+               goto out;
+
+       tracker_class_reset_super_classes (service);
+
+       variant = tracker_ontologies_get_class_value_gvdb (priv->ontologies, priv->uri, "super-classes");
+       if (variant) {
+               GVariantIter iter;
+               const gchar *uri;
+
+               g_variant_iter_init (&iter, variant);
+               while (g_variant_iter_loop (&iter, "&s", &uri)) {
+                       super_class = tracker_ontologies_get_class_by_uri (priv->ontologies, uri);
+
+                       tracker_class_add_super_class (service, super_class);
+               }
+
+               g_variant_unref (variant);
+       }
+
+       priv->use_gvdb = FALSE;
+out:
+       g_mutex_unlock (&priv->mutex);
+}
+
 const gchar *
 tracker_class_get_uri (TrackerClass *service)
 {
@@ -160,27 +202,7 @@ tracker_class_get_super_classes (TrackerClass *service)
 
        priv = tracker_class_get_instance_private (service);
 
-       if (priv->use_gvdb) {
-               TrackerClass *super_class;
-               GVariant *variant;
-
-               tracker_class_reset_super_classes (service);
-
-               variant = tracker_ontologies_get_class_value_gvdb (priv->ontologies, priv->uri, 
"super-classes");
-               if (variant) {
-                       GVariantIter iter;
-                       const gchar *uri;
-
-                       g_variant_iter_init (&iter, variant);
-                       while (g_variant_iter_loop (&iter, "&s", &uri)) {
-                               super_class = tracker_ontologies_get_class_by_uri (priv->ontologies, uri);
-
-                               tracker_class_add_super_class (service, super_class);
-                       }
-
-                       g_variant_unref (variant);
-               }
-       }
+       tracker_class_maybe_sync_from_gvdb (service);
 
        return (TrackerClass **) priv->super_classes->data;
 }
diff --git a/src/libtracker-data/tracker-namespace.c b/src/libtracker-data/tracker-namespace.c
index bb6496e06..6cbb1085e 100644
--- a/src/libtracker-data/tracker-namespace.c
+++ b/src/libtracker-data/tracker-namespace.c
@@ -32,6 +32,7 @@ typedef struct _TrackerNamespacePrivate TrackerNamespacePrivate;
 struct _TrackerNamespacePrivate {
        gchar *uri;
 
+       GMutex mutex;
        guint use_gvdb : 1;
        guint is_new : 1;
 
@@ -54,6 +55,10 @@ tracker_namespace_class_init (TrackerNamespaceClass *klass)
 static void
 tracker_namespace_init (TrackerNamespace *service)
 {
+       TrackerNamespacePrivate *priv;
+
+       priv = tracker_namespace_get_instance_private (service);
+       g_mutex_init (&priv->mutex);
 }
 
 static void
@@ -85,6 +90,29 @@ tracker_namespace_new (gboolean use_gvdb)
        return namespace;
 }
 
+static void
+tracker_namespace_maybe_sync_from_gvdb (TrackerNamespace *namespace)
+{
+       TrackerNamespacePrivate *priv;
+
+       priv = tracker_namespace_get_instance_private (namespace);
+
+       if (!priv->use_gvdb)
+               return;
+
+       g_mutex_lock (&priv->mutex);
+
+       /* In case the lock was contended, make the second lose */
+       if (!priv->use_gvdb)
+               goto out;
+
+       priv->prefix = g_strdup (tracker_ontologies_get_namespace_string_gvdb (priv->ontologies, priv->uri, 
"prefix"));
+
+       priv->use_gvdb = FALSE;
+out:
+       g_mutex_unlock (&priv->mutex);
+}
+
 const gchar *
 tracker_namespace_get_uri (TrackerNamespace *namespace)
 {
@@ -106,9 +134,7 @@ tracker_namespace_get_prefix (TrackerNamespace *namespace)
 
        priv = tracker_namespace_get_instance_private (namespace);
 
-       if (!priv->prefix && priv->use_gvdb) {
-               priv->prefix = g_strdup (tracker_ontologies_get_namespace_string_gvdb (priv->ontologies, 
priv->uri, "prefix"));
-       }
+       tracker_namespace_maybe_sync_from_gvdb (namespace);
 
        return priv->prefix;
 }
diff --git a/src/libtracker-data/tracker-property.c b/src/libtracker-data/tracker-property.c
index 42c7ff931..df72b27cc 100644
--- a/src/libtracker-data/tracker-property.c
+++ b/src/libtracker-data/tracker-property.c
@@ -44,6 +44,7 @@ struct _TrackerPropertyPrivate {
        gchar         *uri;
        gchar         *name;
        gchar         *table_name;
+       GMutex         mutex;
 
        TrackerPropertyType  data_type;
        TrackerClass   *domain;
@@ -145,6 +146,7 @@ tracker_property_init (TrackerProperty *property)
        priv->domain_indexes = g_array_new (TRUE, TRUE, sizeof (TrackerClass *));
        priv->last_super_properties = NULL;
        priv->cardinality_changed = FALSE;
+       g_mutex_init (&priv->mutex);
 }
 
 static void
@@ -207,6 +209,103 @@ tracker_property_new (gboolean use_gvdb)
        return property;
 }
 
+static void
+tracker_property_maybe_sync_from_gvdb (TrackerProperty *property)
+{
+       TrackerPropertyPrivate *priv;
+       GVariant *variant;
+       const gchar *range_uri;
+       const gchar *domain_uri;
+       TrackerClass *domain_index;
+
+       priv = tracker_property_get_instance_private (property);
+
+       if (!priv->use_gvdb)
+               return;
+
+       g_mutex_lock (&priv->mutex);
+
+       /* In case the lock was contended, make the second lose */
+       if (!priv->use_gvdb)
+               goto out;
+
+       /* Data type */
+       range_uri = tracker_ontologies_get_property_string_gvdb (priv->ontologies, priv->uri, "range");
+       if (strcmp (range_uri, XSD_STRING) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_STRING;
+       } else if (strcmp (range_uri, RDF_LANGSTRING) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_LANGSTRING;
+       } else if (strcmp (range_uri, XSD_BOOLEAN) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_BOOLEAN;
+       } else if (strcmp (range_uri, XSD_INTEGER) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_INTEGER;
+       } else if (strcmp (range_uri, XSD_DOUBLE) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_DOUBLE;
+       } else if (strcmp (range_uri, XSD_DATE) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_DATE;
+       } else if (strcmp (range_uri, XSD_DATETIME) == 0) {
+               priv->data_type = TRACKER_PROPERTY_TYPE_DATETIME;
+       } else {
+               priv->data_type = TRACKER_PROPERTY_TYPE_RESOURCE;
+       }
+
+       /* Range */
+       priv->range = g_object_ref (tracker_ontologies_get_class_by_uri (priv->ontologies, range_uri));
+
+       /* Domain */
+       domain_uri = tracker_ontologies_get_property_string_gvdb (priv->ontologies, priv->uri, "domain");
+       priv->domain = g_object_ref (tracker_ontologies_get_class_by_uri (priv->ontologies, domain_uri));
+
+       /* Domain indexes */
+       tracker_property_reset_domain_indexes (property);
+
+       variant = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, "domain-indexes");
+       if (variant) {
+               GVariantIter iter;
+               const gchar *uri;
+
+               g_variant_iter_init (&iter, variant);
+               while (g_variant_iter_loop (&iter, "&s", &uri)) {
+                       domain_index = tracker_ontologies_get_class_by_uri (priv->ontologies, uri);
+
+                       tracker_property_add_domain_index (property, domain_index);
+               }
+
+               g_variant_unref (variant);
+       }
+
+       /* Fulltext indexed */
+       variant = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"fulltext-indexed");
+       if (variant != NULL) {
+               priv->fulltext_indexed = g_variant_get_boolean (variant);
+               g_variant_unref (variant);
+       } else {
+               priv->fulltext_indexed = FALSE;
+       }
+
+       /* Cardinality */
+       variant = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, "max-cardinality");
+       if (variant != NULL) {
+               priv->multiple_values = FALSE;
+               g_variant_unref (variant);
+       } else {
+               priv->multiple_values = TRUE;
+       }
+
+       /* Inverse functional property */
+       variant = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"inverse-functional");
+       if (variant != NULL) {
+               priv->is_inverse_functional_property = g_variant_get_boolean (variant);
+               g_variant_unref (variant);
+       } else {
+               priv->is_inverse_functional_property = FALSE;
+       }
+
+       priv->use_gvdb = FALSE;
+out:
+       g_mutex_unlock (&priv->mutex);
+}
+
 const gchar *
 tracker_property_get_uri (TrackerProperty *property)
 {
@@ -262,28 +361,7 @@ tracker_property_get_data_type (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (priv->use_gvdb) {
-               const gchar *range_uri;
-
-               range_uri = tracker_ontologies_get_property_string_gvdb (priv->ontologies, priv->uri, 
"range");
-               if (strcmp (range_uri, XSD_STRING) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_STRING;
-               } else if (strcmp (range_uri, RDF_LANGSTRING) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_LANGSTRING;
-               } else if (strcmp (range_uri, XSD_BOOLEAN) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_BOOLEAN;
-               } else if (strcmp (range_uri, XSD_INTEGER) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_INTEGER;
-               } else if (strcmp (range_uri, XSD_DOUBLE) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_DOUBLE;
-               } else if (strcmp (range_uri, XSD_DATE) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_DATE;
-               } else if (strcmp (range_uri, XSD_DATETIME) == 0) {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_DATETIME;
-               } else {
-                       priv->data_type = TRACKER_PROPERTY_TYPE_RESOURCE;
-               }
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->data_type;
 }
@@ -300,12 +378,7 @@ tracker_property_get_domain (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (!priv->domain && priv->use_gvdb) {
-               const gchar *domain_uri;
-
-               domain_uri = tracker_ontologies_get_property_string_gvdb (priv->ontologies, priv->uri, 
"domain");
-               priv->domain = g_object_ref (tracker_ontologies_get_class_by_uri (priv->ontologies, 
domain_uri));
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->domain;
 }
@@ -322,27 +395,7 @@ tracker_property_get_domain_indexes (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (priv->use_gvdb) {
-               TrackerClass *domain_index;
-               GVariant *variant;
-
-               tracker_property_reset_domain_indexes (property);
-
-               variant = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"domain-indexes");
-               if (variant) {
-                       GVariantIter iter;
-                       const gchar *uri;
-
-                       g_variant_iter_init (&iter, variant);
-                       while (g_variant_iter_loop (&iter, "&s", &uri)) {
-                               domain_index = tracker_ontologies_get_class_by_uri (priv->ontologies, uri);
-
-                               tracker_property_add_domain_index (property, domain_index);
-                       }
-
-                       g_variant_unref (variant);
-               }
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return (TrackerClass ** ) priv->domain_indexes->data;
 }
@@ -387,12 +440,7 @@ tracker_property_get_range (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (!priv->range && priv->use_gvdb) {
-               const gchar *range_uri;
-
-               range_uri = tracker_ontologies_get_property_string_gvdb (priv->ontologies, priv->uri, 
"range");
-               priv->range = g_object_ref (tracker_ontologies_get_class_by_uri (priv->ontologies, 
range_uri));
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->range;
 }
@@ -457,20 +505,7 @@ tracker_property_get_fulltext_indexed (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (priv->use_gvdb) {
-               GVariant *value;
-               gboolean result;
-
-               value = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"fulltext-indexed");
-               if (value != NULL) {
-                       result = g_variant_get_boolean (value);
-                       g_variant_unref (value);
-               } else {
-                       result = FALSE;
-               }
-
-               return result;
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->fulltext_indexed;
 }
@@ -569,20 +604,7 @@ tracker_property_get_multiple_values (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (priv->use_gvdb) {
-               GVariant *value;
-               gboolean result;
-
-               value = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"max-cardinality");
-               if (value != NULL) {
-                       result = FALSE;
-                       g_variant_unref (value);
-               } else {
-                       result = TRUE;
-               }
-
-               return result;
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->multiple_values;
 }
@@ -620,20 +642,7 @@ tracker_property_get_is_inverse_functional_property (TrackerProperty *property)
 
        priv = tracker_property_get_instance_private (property);
 
-       if (priv->use_gvdb) {
-               GVariant *value;
-               gboolean result;
-
-               value = tracker_ontologies_get_property_value_gvdb (priv->ontologies, priv->uri, 
"inverse-functional");
-               if (value != NULL) {
-                       result = g_variant_get_boolean (value);
-                       g_variant_unref (value);
-               } else {
-                       result = FALSE;
-               }
-
-               return result;
-       }
+       tracker_property_maybe_sync_from_gvdb (property);
 
        return priv->is_inverse_functional_property;
 }