[epiphany] history-service: Ensure thread member is initialized before use



commit 594e489735f0967dd582a664a759abfb0a2828a0
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Sat Feb 18 21:28:22 2017 -0600

    history-service: Ensure thread member is initialized before use
    
    We have assertions to ensure that several functions are only ever called
    on the history thread. But the first such assertion, at the top of
    run_history_service_thread, sometimes fails when running the tests. It
    is racy. Use a mutex to fix this.
    
    These assertions are actually executed at runtime for end users, so it's
    surprising that nobody has ever reported a bug about this.
    
    We also need to be sure to initialize the async queue before running the
    history service thread. The mutex is needed as a memory barrier here, so
    it's not possible to remove the mutex by removing the assertions except
    in debug mode, which is something I considered.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778649

 lib/history/ephy-history-service-private.h |    1 +
 lib/history/ephy-history-service.c         |   15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)
---
diff --git a/lib/history/ephy-history-service-private.h b/lib/history/ephy-history-service-private.h
index 9555479..f581e94 100644
--- a/lib/history/ephy-history-service-private.h
+++ b/lib/history/ephy-history-service-private.h
@@ -28,6 +28,7 @@ struct _EphyHistoryService {
   GObject parent_instance;
   char *history_filename;
   EphySQLiteConnection *history_database;
+  GMutex history_thread_mutex;
   GThread *history_thread;
   GAsyncQueue *queue;
   gboolean scheduled_to_quit;
diff --git a/lib/history/ephy-history-service.c b/lib/history/ephy-history-service.c
index becdb73..2b32caa 100644
--- a/lib/history/ephy-history-service.c
+++ b/lib/history/ephy-history-service.c
@@ -268,8 +268,14 @@ ephy_history_service_class_init (EphyHistoryServiceClass *klass)
 static void
 ephy_history_service_init (EphyHistoryService *self)
 {
-  self->history_thread = g_thread_new ("EphyHistoryService", (GThreadFunc)run_history_service_thread, self);
   self->queue = g_async_queue_new ();
+
+  /* This value is checked in several functions to verify that they are only
+   * ever run on the history thread. Accordingly, we'd better be sure it's set
+   * before it is checked for the first time. That requires a lock here. */
+  g_mutex_lock (&self->history_thread_mutex);
+  self->history_thread = g_thread_new ("EphyHistoryService", (GThreadFunc)run_history_service_thread, self);
+  g_mutex_unlock (&self->history_thread_mutex);
 }
 
 EphyHistoryService *
@@ -489,7 +495,14 @@ run_history_service_thread (EphyHistoryService *self)
 {
   EphyHistoryServiceMessage *message;
 
+  /* Note that self->history_thread is only written once, and that's guaranteed
+   * to have occurred before we enter this critical section due to this mutex.
+   * Accordingly, we do not need to use the mutex when performing these
+   * assertions in other functions.
+   */
+  g_mutex_lock (&self->history_thread_mutex);
   g_assert (self->history_thread == g_thread_self ());
+  g_mutex_unlock (&self->history_thread_mutex);
 
   if (ephy_history_service_open_database_connections (self) == FALSE)
     return NULL;


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