[evolution-data-server] Prevent early free of an ESource when it has pending operations



commit 926d6992668095f2679051a10a13499b2134f180
Author: Milan Crha <mcrha redhat com>
Date:   Thu Jan 11 14:48:52 2018 +0100

    Prevent early free of an ESource when it has pending operations
    
    It could happen, in some situations, that an ESource could be freed
    in a dedicated thread, while it had been in use in an idle callback.
    As it held a corresponding mutex, the crash happened in the finalize()
    function, when it had been tried to free that locked mutex.
    
    The change removes obsolete members from the private structure of
    the ESource and references itself for the idle callbacks. Even it
    avoids early free (and idle callback cancellation in the dispose()),
    it will make sure that the ESource will not disappear while the idle
    callback is running, the same as it'll avoid some other races around
    this situation.
    
    It had been reported downstream at:
    https://bugzilla.redhat.com/show_bug.cgi?id=1533442

 src/libedataserver/e-source.c |   26 +++++++++++---------------
 1 files changed, 11 insertions(+), 15 deletions(-)
---
diff --git a/src/libedataserver/e-source.c b/src/libedataserver/e-source.c
index 8b99191..3da1b66 100644
--- a/src/libedataserver/e-source.c
+++ b/src/libedataserver/e-source.c
@@ -137,9 +137,6 @@ struct _ESourcePrivate {
        GMutex connection_status_change_lock;
        ESourceConnectionStatus connection_status;
 
-       GSource *credentials_required_call;
-       GMutex credentials_required_call_lock;
-
        GMutex property_lock;
 
        gchar *display_name;
@@ -913,7 +910,7 @@ source_notify_dbus_connection_status_cb (EDBusSource *dbus_source,
                g_source_set_callback (
                        source->priv->connection_status_change,
                        source_idle_connection_status_change_cb,
-                       source, NULL);
+                       g_object_ref (source), g_object_unref);
                g_source_attach (
                        source->priv->connection_status_change,
                        source->priv->main_context);
@@ -1227,6 +1224,13 @@ source_dispose (GObject *object)
 
        priv = E_SOURCE_GET_PRIVATE (object);
 
+       /* Lock & unlock to make sure any pending operations in other threads
+          which use this lock are already done */
+       g_rec_mutex_lock (&priv->lock);
+       g_rec_mutex_unlock (&priv->lock);
+
+       g_mutex_lock (&priv->property_lock);
+
        if (priv->dbus_object != NULL) {
                EDBusObject *dbus_object;
                EDBusSource *dbus_source;
@@ -1245,6 +1249,8 @@ source_dispose (GObject *object)
                priv->dbus_object = NULL;
        }
 
+       g_mutex_unlock (&priv->property_lock);
+
        if (priv->main_context != NULL) {
                g_main_context_unref (priv->main_context);
                priv->main_context = NULL;
@@ -1267,14 +1273,6 @@ source_dispose (GObject *object)
        }
        g_mutex_unlock (&priv->connection_status_change_lock);
 
-       g_mutex_lock (&priv->credentials_required_call_lock);
-       if (priv->credentials_required_call != NULL) {
-               g_source_destroy (priv->credentials_required_call);
-               g_source_unref (priv->credentials_required_call);
-               priv->credentials_required_call = NULL;
-       }
-       g_mutex_unlock (&priv->credentials_required_call_lock);
-
        g_hash_table_remove_all (priv->extensions);
 
        /* Chain up to parent's dispose() method. */
@@ -1290,7 +1288,6 @@ source_finalize (GObject *object)
 
        g_mutex_clear (&priv->changed_lock);
        g_mutex_clear (&priv->connection_status_change_lock);
-       g_mutex_clear (&priv->credentials_required_call_lock);
        g_mutex_clear (&priv->property_lock);
 
        g_free (priv->display_name);
@@ -2436,7 +2433,6 @@ e_source_init (ESource *source)
        source->priv = E_SOURCE_GET_PRIVATE (source);
        g_mutex_init (&source->priv->changed_lock);
        g_mutex_init (&source->priv->connection_status_change_lock);
-       g_mutex_init (&source->priv->credentials_required_call_lock);
        g_mutex_init (&source->priv->property_lock);
        source->priv->key_file = g_key_file_new ();
        source->priv->extensions = extensions;
@@ -2617,7 +2613,7 @@ e_source_changed (ESource *source)
                g_source_set_callback (
                        source->priv->changed,
                        source_idle_changed_cb,
-                       source, (GDestroyNotify) NULL);
+                       g_object_ref (source), g_object_unref);
                g_source_attach (
                        source->priv->changed,
                        source->priv->main_context);


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