[nautilus] search-engine-simple: Make some modifications to previous commit

[Carlos Soriano <csoriano src gnome org>]

commit 3a539ef131982d05454845d7ab7757a9fd1aedf2
Author: Carlos Soriano <csoriano redhat com>
Date:   Thu Feb 21 16:52:30 2019 +0100

    search-engine-simple: Make some modifications to previous commit
    
    * Cleaner separation of phases
    * Avoid some races
    * Fix some leaks

 src/nautilus-search-engine-simple.c | 131 +++++++++++++++++-------------------
 1 file changed, 62 insertions(+), 69 deletions(-)
---
diff --git a/src/nautilus-search-engine-simple.c b/src/nautilus-search-engine-simple.c
index 1318cbdec..1687d218a 100644
--- a/src/nautilus-search-engine-simple.c
+++ b/src/nautilus-search-engine-simple.c
@@ -59,8 +59,12 @@ typedef struct
 
     NautilusQuery *query;
 
+    gint processing_id;
     GMutex idle_mutex;
+    // The following data can be accessed from different threads
+    // and needs to lock the mutex
     GQueue *idle_queue;
+    gboolean finished;
 } SearchThreadData;
 
 
@@ -119,6 +123,8 @@ search_thread_data_new (NautilusSearchEngineSimple *engine,
 static void
 search_thread_data_free (SearchThreadData *data)
 {
+    GList *hits;
+
     g_queue_foreach (data->directories,
                      (GFunc) g_object_unref, NULL);
     g_queue_free (data->directories);
@@ -129,13 +135,18 @@ search_thread_data_free (SearchThreadData *data)
     g_list_free_full (data->hits, g_object_unref);
     g_object_unref (data->engine);
     g_mutex_clear (&data->idle_mutex);
+
+    while ((hits = g_queue_pop_head (data->idle_queue)))
+    {
+        g_list_free_full (hits, g_object_unref);
+    }
     g_queue_free (data->idle_queue);
 
     g_free (data);
 }
 
-static void
-search_thread_done_idle (SearchThreadData *data)
+static gboolean
+search_thread_done (SearchThreadData *data)
 {
     NautilusSearchEngineSimple *engine = data->engine;
 
@@ -154,6 +165,8 @@ search_thread_done_idle (SearchThreadData *data)
     g_object_notify (G_OBJECT (engine), "running");
 
     search_thread_data_free (data);
+
+    return G_SOURCE_REMOVE;
 }
 
 static void
@@ -167,108 +180,88 @@ search_thread_process_hits_idle (SearchThreadData *data, GList *hits)
     }
 }
 
-typedef struct
-{
-    gboolean clean_up;  /* Whether we are done and should clean up */
-    GList *hits;        /* Must be a valid list when clean_up is not set,
-                         * otherwise unused */
-} IdleQueueData;
-
 static gboolean
-search_thread_idle (gpointer user_data)
+search_thread_process_idle (gpointer user_data)
 {
     SearchThreadData *thread_data;
-    IdleQueueData *queue_data;
-    gboolean is_last;
+    GList *hits;
 
     thread_data = user_data;
 
     g_mutex_lock (&thread_data->idle_mutex);
-    queue_data = g_queue_pop_head (thread_data->idle_queue);
-    if (queue_data)
+    hits = g_queue_pop_head (thread_data->idle_queue);
+    // Even if the cancellable is cancelled, we need to make sure the search
+    // thread has aknowledge it, and therefore not using the thread data after
+    // freeing it. The search thread will mark as finished whenever the search
+    // is finished or cancelled.
+    // Nonetheless, we should stop yielding results if the search was cancelled
+    if (thread_data->finished)
     {
-        is_last = g_queue_is_empty (thread_data->idle_queue);
-        g_mutex_unlock (&thread_data->idle_mutex);
-
-        if (!queue_data->clean_up)
+        if (hits == NULL || g_cancellable_is_cancelled (thread_data->cancellable))
         {
-            search_thread_process_hits_idle (thread_data, queue_data->hits);
-            g_list_free_full (queue_data->hits, g_object_unref);
-            g_free (queue_data);
+          g_mutex_unlock (&thread_data->idle_mutex);
 
-            return is_last ? G_SOURCE_REMOVE : G_SOURCE_CONTINUE;
-        }
-        else
-        {
-            search_thread_done_idle (thread_data);
-            g_free (queue_data);
+          if (hits)
+          {
+              g_list_free_full (hits, g_object_unref);
+          }
+          search_thread_done (thread_data);
+
+          return G_SOURCE_REMOVE;
         }
+
     }
-    else
+
+    g_mutex_unlock (&thread_data->idle_mutex);
+
+    if (hits)
     {
-        g_mutex_unlock (&thread_data->idle_mutex);
+        search_thread_process_hits_idle (thread_data, hits);
+        g_list_free_full (hits, g_object_unref);
     }
 
-    return G_SOURCE_REMOVE;
+    return G_SOURCE_CONTINUE;
 }
 
-/* Do not directly call this function.
- * Use start_idle_processing or start_idle_cleanup instead. */
 static void
-start_idle_processing_or_cleanup (SearchThreadData *thread_data,
-                                  GList            *hits,
-                                  gboolean          clean_up)
+finish_search_thread (SearchThreadData *thread_data)
 {
-    gboolean idle_stopped;
-    IdleQueueData *queue_data;
-
-    g_assert (hits || clean_up);
-
     g_mutex_lock (&thread_data->idle_mutex);
-    idle_stopped = g_queue_is_empty (thread_data->idle_queue);
-    if (hits)
-    {
-        queue_data = g_new (IdleQueueData, 1);
-        queue_data->clean_up = FALSE;
-        queue_data->hits = hits;
-        g_queue_push_tail (thread_data->idle_queue, queue_data);
-    }
-    if (clean_up)
-    {
-        queue_data = g_new (IdleQueueData, 1);
-        queue_data->clean_up = TRUE;
-        queue_data->hits = NULL;
-        g_queue_push_tail (thread_data->idle_queue, queue_data);
-    }
+    thread_data->finished = TRUE;
     g_mutex_unlock (&thread_data->idle_mutex);
 
-    if (idle_stopped)
+    // If no results were processed, direclty finish the search, in the main
+    // thread.
+    if (thread_data->processing_id == 0)
     {
-        g_idle_add (search_thread_idle, thread_data);
+        g_idle_add (G_SOURCE_FUNC (search_thread_done), thread_data);
     }
 }
 
 static void
-start_idle_processing (SearchThreadData *thread_data,
+process_batch_in_idle (SearchThreadData *thread_data,
                        GList            *hits)
 {
-    start_idle_processing_or_cleanup (thread_data, hits, FALSE);
-}
+    g_return_if_fail (hits != NULL);
 
-static void
-start_idle_cleanup (SearchThreadData *thread_data)
-{
-    start_idle_processing_or_cleanup (thread_data, NULL, TRUE);
+    g_mutex_lock (&thread_data->idle_mutex);
+    g_queue_push_tail (thread_data->idle_queue, hits);
+    g_mutex_unlock (&thread_data->idle_mutex);
+
+    if (thread_data->processing_id == 0)
+    {
+        thread_data->processing_id = g_idle_add (search_thread_process_idle, thread_data);
+    }
 }
 
 static void
-send_batch (SearchThreadData *thread_data)
+send_batch_in_idle (SearchThreadData *thread_data)
 {
     thread_data->n_processed_files = 0;
 
     if (thread_data->hits)
     {
-        start_idle_processing (thread_data, thread_data->hits);
+        process_batch_in_idle (thread_data, thread_data->hits);
     }
     thread_data->hits = NULL;
 }
@@ -398,7 +391,7 @@ visit_directory (GFile            *dir,
         data->n_processed_files++;
         if (data->n_processed_files > BATCH_SIZE)
         {
-            send_batch (data);
+            send_batch_in_idle (data);
         }
 
         if (recursive != NAUTILUS_QUERY_RECURSIVE_NEVER &&
@@ -468,10 +461,10 @@ search_thread_func (gpointer user_data)
 
     if (!g_cancellable_is_cancelled (data->cancellable))
     {
-        send_batch (data);
+        send_batch_in_idle (data);
     }
 
-    start_idle_cleanup (data);
+    finish_search_thread (data);
 
     return NULL;
 }