[epiphany/gnome-3-18] history-service: Ensure thread member is initialized before use



commit 1c7e776b431929e8251613cfedb2362704f9a0b1
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 8f059ec..977fa2e 100644
--- a/lib/history/ephy-history-service-private.h
+++ b/lib/history/ephy-history-service-private.h
@@ -25,6 +25,7 @@
 struct _EphyHistoryServicePrivate {
   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 3af20da..5b14a9d 100644
--- a/lib/history/ephy-history-service.c
+++ b/lib/history/ephy-history-service.c
@@ -290,8 +290,14 @@ ephy_history_service_init (EphyHistoryService *self)
 {
   self->priv = EPHY_HISTORY_SERVICE_GET_PRIVATE (self);
 
-  self->priv->history_thread = g_thread_new ("EphyHistoryService", (GThreadFunc) run_history_service_thread, 
self);
   self->priv->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->priv->history_thread_mutex);
+  self->priv->history_thread = g_thread_new ("EphyHistoryService", (GThreadFunc)run_history_service_thread, 
self);
+  g_mutex_unlock (&self->priv->history_thread_mutex);
 }
 
 EphyHistoryService *
@@ -516,7 +522,14 @@ run_history_service_thread (EphyHistoryService *self)
   EphyHistoryServicePrivate *priv = self->priv;
   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 (&priv->history_thread_mutex);
   g_assert (priv->history_thread == g_thread_self ());
+  g_mutex_unlock (&priv->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]