[glib-networking/mcatanzaro/#85] base: fix invalid push/pop of handshake_context



commit 5e7568189d16f530a77f7e0fd5d71d1e1d216b27
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Fri May 17 16:29:52 2019 -0500

    base: fix invalid push/pop of handshake_context
    
    If handshake_context is reused -- i.e., it is assigned to using
    g_main_context_ref_thread_default() rather than g_main_context_new() and
    g_main_context_push_thread_default() -- then we do not need to, and must
    not, pop it before emitting the accept-certificate callback. I was
    thinking that reffing the thread default context would increase its
    "reference count" on the stack, such that the pop call here would be
    paired with the ref and ultimately ignored. That's absolutely not how it
    works.
    
    We'll invert the meaning of sync_handshake_completed to become
    sync_handshake_in_progress so that we can reuse it here.
    
    Fixes #85

 tls/base/gtlsconnection-base.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 76defda..69f5718 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -123,7 +123,7 @@ typedef struct
    */
   gboolean       need_handshake;
   gboolean       need_finish_handshake;
-  gboolean       sync_handshake_completed;
+  gboolean       sync_handshake_in_progress;
   gboolean       started_handshake;
   gboolean       handshaking;
   gboolean       ever_handshaked;
@@ -1254,11 +1254,19 @@ accept_or_reject_peer_certificate (gpointer user_data)
 
   if (!accepted)
     {
-      g_main_context_pop_thread_default (priv->handshake_context);
+      g_mutex_lock (&priv->op_mutex);
+      if (priv->sync_handshake_in_progress)
+        g_main_context_pop_thread_default (priv->handshake_context);
+      g_mutex_unlock (&priv->op_mutex);
+
       accepted = g_tls_connection_emit_accept_certificate (G_TLS_CONNECTION (tls),
                                                            priv->peer_certificate,
                                                            priv->peer_certificate_errors);
-      g_main_context_push_thread_default (priv->handshake_context);
+
+      g_mutex_lock (&priv->op_mutex);
+      if (priv->sync_handshake_in_progress)
+        g_main_context_push_thread_default (priv->handshake_context);
+      g_mutex_unlock (&priv->op_mutex);
     }
 
   priv->peer_certificate_accepted = accepted;
@@ -1407,7 +1415,7 @@ sync_handshake_thread_completed (GObject      *object,
   g_assert (g_main_context_is_owner (priv->handshake_context));
 
   g_mutex_lock (&priv->op_mutex);
-  priv->sync_handshake_completed = TRUE;
+  priv->sync_handshake_in_progress = FALSE;
   g_mutex_unlock (&priv->op_mutex);
 
   g_main_context_wakeup (priv->handshake_context);
@@ -1424,8 +1432,8 @@ crank_sync_handshake_context (GTlsConnectionBase *tls,
    * here. So need_finish_handshake should only change on this thread.
    */
   g_mutex_lock (&priv->op_mutex);
-  priv->sync_handshake_completed = FALSE;
-  while (!priv->sync_handshake_completed && !g_cancellable_is_cancelled (cancellable))
+  priv->sync_handshake_in_progress = TRUE;
+  while (priv->sync_handshake_in_progress && !g_cancellable_is_cancelled (cancellable))
     {
       g_mutex_unlock (&priv->op_mutex);
       g_main_context_iteration (priv->handshake_context, TRUE);


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