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



commit 759897584c6a837b7d8924df5853901b4e1bb7a7
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 | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index bc04e15..44ecbdd 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,21 @@ accept_or_reject_peer_certificate (gpointer user_data)
 
   if (!accepted)
     {
-      g_main_context_pop_thread_default (priv->handshake_context);
+      gboolean sync_handshake_in_progress;
+
+      g_mutex_lock (&priv->op_mutex);
+      sync_handshake_in_progress = priv->sync_handshake_in_progress;
+      g_mutex_unlock (&priv->op_mutex);
+
+      if (sync_handshake_in_progress)
+        g_main_context_pop_thread_default (priv->handshake_context);
+
       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);
+
+      if (sync_handshake_in_progress)
+        g_main_context_push_thread_default (priv->handshake_context);
     }
 
   priv->peer_certificate_accepted = accepted;
@@ -1407,7 +1417,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 +1434,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]