[tracker-miners/wip/carlosg/test-error-fixes: 5/5] libtracker-miner: Fix TrackerMinerFS event coalescing




commit 4eaaf73dc4a0590d58a528a4169c1d76369d501c
Author: Carlos Garnacho <carlosg gnome org>
Date:   Tue Jul 13 11:56:13 2021 +0200

    libtracker-miner: Fix TrackerMinerFS event coalescing
    
    This infrastructure relied on GFiles being unique-ified by the
    TrackerFileSystem. Since we remove that piece of caching, GFiles
    for the same path can actually be different instances at this
    level, so we should account for this.
    
    Keep a hash table using GFile equal/hash functions, so we can
    still have this event coalescing in place.
    
    The way this was missed feels rather disconnected, when a directory
    and a child file are created in a quick sucession, it may happen
    that the TrackerFileNotifier did add a monitor for the directory,
    started crawling it, and found this new file on both paths. If
    this happens, the 2 CREATED events will be propagated upwards.
    
    Since crawling+monitoring is inherently racy (and inverting the
    order may actually miss events), we have to deal with this double
    emission since we cannot handle it within TrackerFileNotifier.
    This was usually handled at the TrackerMinerFS event queue level
    by this event coalescing.
    
    This situation was noticed during investigation of
    https://gitlab.gnome.org/GNOME/tracker-miners/-/issues/179, there
    are additional issues in the Tracker library leading to the warning,
    but this is first thing that starts going wrong there, and this
    fix is sufficient to avoid the issue entirely.
    
    Fixes: https://gitlab.gnome.org/GNOME/tracker-miners/-/issues/179

 src/libtracker-miner/tracker-miner-fs.c | 77 +++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 33 deletions(-)
---
diff --git a/src/libtracker-miner/tracker-miner-fs.c b/src/libtracker-miner/tracker-miner-fs.c
index a7260ec2e..f496f45af 100644
--- a/src/libtracker-miner/tracker-miner-fs.c
+++ b/src/libtracker-miner/tracker-miner-fs.c
@@ -108,6 +108,7 @@ typedef struct {
 
 struct _TrackerMinerFSPrivate {
        TrackerPriorityQueue *items;
+       GHashTable *items_by_file;
 
        guint item_queues_handler_id;
        GFile *item_queue_blocker;
@@ -440,6 +441,8 @@ tracker_miner_fs_init (TrackerMinerFS *object)
        priv->extraction_timer_stopped = TRUE;
 
        priv->items = tracker_priority_queue_new ();
+       priv->items_by_file = g_hash_table_new (g_file_hash,
+                                               (GEqualFunc) g_file_equal);
 
        /* Create processing pools */
        priv->task_pool = tracker_task_pool_new (DEFAULT_WAIT_POOL_LIMIT);
@@ -605,35 +608,6 @@ queue_event_moved_new (GFile    *source,
        return event;
 }
 
-static GList *
-queue_event_get_last_event_node (QueueEvent *event)
-{
-       return g_object_get_qdata (G_OBJECT (event->file),
-                                  quark_last_queue_event);
-}
-
-static void
-queue_event_save_node (QueueEvent *event,
-                      GList      *node)
-{
-       g_assert (node->data == event);
-       g_object_set_qdata (G_OBJECT (event->file),
-                           quark_last_queue_event, node);
-}
-
-static void
-queue_event_dispose_node (QueueEvent *event)
-{
-       GList *node;
-
-       node = queue_event_get_last_event_node (event);
-
-       if (node && node->data == event) {
-               g_object_steal_qdata (G_OBJECT (event->file),
-                                     quark_last_queue_event);
-       }
-}
-
 static void
 queue_event_free (QueueEvent *event)
 {
@@ -644,8 +618,6 @@ queue_event_free (QueueEvent *event)
                g_queue_delete_link (root_queue, event->root_node);
        }
 
-       queue_event_dispose_node (event);
-
        g_clear_object (&event->dest_file);
        g_clear_object (&event->file);
        g_clear_object (&event->info);
@@ -755,6 +727,7 @@ fs_finalize (GObject *object)
                g_object_unref (priv->sparql_buffer);
        }
 
+       g_hash_table_unref (priv->items_by_file);
        tracker_priority_queue_foreach (priv->items,
                                        (GFunc) queue_event_free,
                                        NULL);
@@ -1280,6 +1253,33 @@ should_wait (TrackerMinerFS *fs,
        return FALSE;
 }
 
+static gboolean
+maybe_remove_file_event_node (TrackerMinerFS *fs,
+                              QueueEvent     *event)
+{
+       GList *link;
+
+       link = g_hash_table_lookup (fs->priv->items_by_file, event->file);
+
+       if (link && link->data == event) {
+               g_hash_table_remove (fs->priv->items_by_file, event->file);
+               return TRUE;
+       }
+
+       return FALSE;
+}
+
+static gboolean
+remove_items_by_file_foreach (gpointer key,
+                              gpointer value,
+                              gpointer user_data)
+{
+       GList *link = value;
+       GFile *file = user_data;
+
+       return queue_event_is_equal_or_descendant (link->data, file);
+}
+
 static gboolean
 item_queue_get_next_file (TrackerMinerFS           *fs,
                           GFile                   **file,
@@ -1329,6 +1329,7 @@ item_queue_get_next_file (TrackerMinerFS           *fs,
                *is_dir = event->is_dir;
                g_set_object (info, event->info);
 
+               maybe_remove_file_event_node (fs, event);
                queue_event_free (event);
                tracker_priority_queue_pop (fs->priv->items, NULL);
        }
@@ -1698,13 +1699,16 @@ miner_fs_queue_event (TrackerMinerFS *fs,
 
        if (event->type == TRACKER_MINER_FS_EVENT_MOVED) {
                /* Remove all children of the dest location from being processed. */
+               g_hash_table_foreach_remove (fs->priv->items_by_file,
+                                            remove_items_by_file_foreach,
+                                            event->dest_file);
                tracker_priority_queue_foreach_remove (fs->priv->items,
                                                       (GEqualFunc) queue_event_is_equal_or_descendant,
                                                       event->dest_file,
                                                       (GDestroyNotify) queue_event_free);
        }
 
-       old = queue_event_get_last_event_node (event);
+       old = g_hash_table_lookup (fs->priv->items_by_file, event->file);
 
        if (old) {
                QueueCoalesceAction action;
@@ -1713,6 +1717,7 @@ miner_fs_queue_event (TrackerMinerFS *fs,
                action = queue_event_coalesce (old->data, event, &replacement);
 
                if (action & QUEUE_ACTION_DELETE_FIRST) {
+                       maybe_remove_file_event_node (fs, old->data);
                        queue_event_free (old->data);
                        tracker_priority_queue_remove_node (fs->priv->items,
                                                            old);
@@ -1730,6 +1735,9 @@ miner_fs_queue_event (TrackerMinerFS *fs,
        if (event) {
                if (event->type == TRACKER_MINER_FS_EVENT_DELETED) {
                        /* Remove all children of this file from being processed. */
+                       g_hash_table_foreach_remove (fs->priv->items_by_file,
+                                                    remove_items_by_file_foreach,
+                                                    event->file);
                        tracker_priority_queue_foreach_remove (fs->priv->items,
                                                               (GEqualFunc) 
queue_event_is_equal_or_descendant,
                                                               event->file,
@@ -1740,7 +1748,7 @@ miner_fs_queue_event (TrackerMinerFS *fs,
 
                assign_root_node (fs, event);
                link = tracker_priority_queue_add (fs->priv->items, event, priority);
-               queue_event_save_node (event, link);
+               g_hash_table_replace (fs->priv->items_by_file, event->file, link);
                item_queue_handlers_set_up (fs);
        }
 }
@@ -1937,6 +1945,9 @@ indexing_tree_directory_removed (TrackerIndexingTree *indexing_tree,
        /* Remove anything contained in the removed directory
         * from all relevant processing queues.
         */
+       g_hash_table_foreach_remove (fs->priv->items_by_file,
+                                    remove_items_by_file_foreach,
+                                    directory);
        tracker_priority_queue_foreach_remove (priv->items,
                                               (GEqualFunc) queue_event_is_equal_or_descendant,
                                               directory,


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