soup_session_abort: fix a race condition in SoupSessionSync

soup_session_abort() on a SoupSessionSync would sometimes crash,
because of races between the cancellation of the pending messages and
the destruction of the open connections. Fix this by waiting for all
of the messages to finish cancelling before we start closing the
connections.

diff --git a/libsoup/soup-session-sync.c b/libsoup/soup-session-sync.c
index 493d8c5..8cc7ab5 100644 (file)
--- a/libsoup/soup-session-sync.c
+++ b/libsoup/soup-session-sync.c
@@ -60,6 +60,7 @@ static void  cancel_message (SoupSession *session, SoupMessage *msg,
                             guint status_code);
 static void  auth_required  (SoupSession *session, SoupMessage *msg,
                             SoupAuth *auth, gboolean retrying);
+static void  flush_queue    (SoupSession *session);
 
 G_DEFINE_TYPE (SoupSessionSync, soup_session_sync, SOUP_TYPE_SESSION)
 
@@ -96,6 +97,8 @@ soup_session_sync_class_init (SoupSessionSyncClass *session_sync_class)
        session_class->send_message = send_message;
        session_class->cancel_message = cancel_message;
        session_class->auth_required = auth_required;
+       session_class->flush_queue = flush_queue;
+
        object_class->finalize = finalize;
 }
 
@@ -399,3 +402,46 @@ auth_required (SoupSession *session, SoupMessage *msg,
        SOUP_SESSION_CLASS (soup_session_sync_parent_class)->
                auth_required (session, msg, auth, retrying);
 }
+
+static void
+flush_queue (SoupSession *session)
+{
+       SoupSessionSyncPrivate *priv = SOUP_SESSION_SYNC_GET_PRIVATE (session);
+       SoupMessageQueue *queue;
+       SoupMessageQueueItem *item;
+       GHashTable *current;
+       gboolean done = FALSE;
+
+       /* Record the current contents of the queue */
+       current = g_hash_table_new (NULL, NULL);
+       queue = soup_session_get_queue (session);
+       for (item = soup_message_queue_first (queue);
+            item;
+            item = soup_message_queue_next (queue, item))
+               g_hash_table_insert (current, item, item);
+
+       /* Cancel everything */
+       SOUP_SESSION_CLASS (soup_session_sync_parent_class)->flush_queue (session);
+
+       /* Wait until all of the items in @current have been removed
+        * from the queue. (This is not the same as "wait for the
+        * queue to be empty", because the app may queue new requests
+        * in response to the cancellation of the old ones. We don't
+        * try to cancel those requests as well, since we'd likely
+        * just end up looping forever.)
+        */
+       g_mutex_lock (priv->lock);
+       do {
+               done = TRUE;
+               for (item = soup_message_queue_first (queue);
+                    item;
+                    item = soup_message_queue_next (queue, item)) {
+                       if (g_hash_table_lookup (current, item))
+                               done = FALSE;
+               }
+
+               if (!done)
+                       g_cond_wait (priv->cond, priv->lock);
+       } while (!done);
+       g_mutex_unlock (priv->lock);
+}

diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index 4f9d975..ba98e65 100644 (file)
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -111,6 +111,7 @@ static void cancel_message  (SoupSession *session, SoupMessage *msg,
                             guint status_code);
 static void auth_required   (SoupSession *session, SoupMessage *msg,
                             SoupAuth *auth, gboolean retrying);
+static void flush_queue     (SoupSession *session);
 
 static void auth_manager_authenticate (SoupAuthManager *manager,
                                       SoupMessage *msg, SoupAuth *auth,
@@ -257,6 +258,7 @@ soup_session_class_init (SoupSessionClass *session_class)
        session_class->requeue_message = requeue_message;
        session_class->cancel_message = cancel_message;
        session_class->auth_required = auth_required;
+       session_class->flush_queue = flush_queue;
 
        /* virtual method override */
        object_class->dispose = dispose;
@@ -1751,6 +1753,20 @@ gather_conns (gpointer key, gpointer host, gpointer data)
        *conns = g_slist_prepend (*conns, g_object_ref (conn));
 }
 
+static void
+flush_queue (SoupSession *session)
+{
+       SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
+       SoupMessageQueueItem *item;
+
+       for (item = soup_message_queue_first (priv->queue);
+            item;
+            item = soup_message_queue_next (priv->queue, item)) {
+               soup_session_cancel_message (session, item->msg,
+                                            SOUP_STATUS_CANCELLED);
+       }
+}
+
 /**
  * soup_session_abort:
  * @session: the session
@@ -1761,18 +1777,12 @@ void
 soup_session_abort (SoupSession *session)
 {
        SoupSessionPrivate *priv;
-       SoupMessageQueueItem *item;
        GSList *conns, *c;
 
        g_return_if_fail (SOUP_IS_SESSION (session));
        priv = SOUP_SESSION_GET_PRIVATE (session);
 
-       for (item = soup_message_queue_first (priv->queue);
-            item;
-            item = soup_message_queue_next (priv->queue, item)) {
-               soup_session_cancel_message (session, item->msg,
-                                            SOUP_STATUS_CANCELLED);
-       }
+       SOUP_SESSION_GET_CLASS (session)->flush_queue (session);
 
        /* Close all connections */
        g_mutex_lock (priv->host_lock);

diff --git a/libsoup/soup-session.h b/libsoup/soup-session.h
index fe07892..4b6661f 100644 (file)
--- a/libsoup/soup-session.h
+++ b/libsoup/soup-session.h
@@ -49,9 +49,9 @@ typedef struct {
        void  (*auth_required)   (SoupSession *session, SoupMessage *msg,
                                  SoupAuth *auth, gboolean retrying);
 
+       void  (*flush_queue)     (SoupSession *session);
 
        /* Padding for future expansion */
-       void (*_libsoup_reserved2) (void);
        void (*_libsoup_reserved3) (void);
        void (*_libsoup_reserved4) (void);
 } SoupSessionClass;