[libsoup] soup_session_abort: fix a race condition in SoupSessionSync

[Dan Winship <danw src gnome org>]

commit 4fccc7a2464075e83e572a7907cc11831cddd872
Author: Dan Winship <danw gnome org>
Date:   Wed Apr 20 10:18:57 2011 -0400

    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.

 libsoup/soup-session-sync.c |   46 +++++++++++++++++++++++++++++++++++++++++++
 libsoup/soup-session.c      |   24 +++++++++++++++------
 libsoup/soup-session.h      |    2 +-
 3 files changed, 64 insertions(+), 8 deletions(-)
---
diff --git a/libsoup/soup-session-sync.c b/libsoup/soup-session-sync.c
index 493d8c5..8cc7ab5 100644
--- 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
--- 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
--- 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;