If the connection attempt succeeded, reserve the connection before

        * libsoup/soup-session.c (connect_result): If the connection
        attempt succeeded, reserve the connection before releasing
        host_lock. Otherwise, another thread might find it in the
        connection pool before the caller can queue a message on it.
        #60693

        * libsoup/soup-session-async.c (got_connection): Call
        soup_connection_release(), since we don't have a specific message
        in mind for the connection, so we need it to be considered idle.

        * libsoup/soup-connection.c (soup_connection_release): New
        function, to undo a soup_connection_reserve().
        (soup_connection_send_request, soup_connection_reserve,
        soup_connection_authenticate, soup_connection_reauthenticate):
        Document these

5 files changed, 130 insertions, 19 deletions

diff --git a/ChangeLog b/ChangeLog
index f902116c..12c06f26 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2004-07-13  Dan Winship  <danw@novell.com>
+
+	* libsoup/soup-session.c (connect_result): If the connection
+	attempt succeeded, reserve the connection before releasing
+	host_lock. Otherwise, another thread might find it in the
+	connection pool before the caller can queue a message on it.
+	#60693
+
+	* libsoup/soup-session-async.c (got_connection): Call
+	soup_connection_release(), since we don't have a specific message
+	in mind for the connection, so we need it to be considered idle.
+
+	* libsoup/soup-connection.c (soup_connection_release): New
+	function, to undo a soup_connection_reserve().
+	(soup_connection_send_request, soup_connection_reserve,
+	soup_connection_authenticate, soup_connection_reauthenticate):
+	Document these
+
 2004-07-12  Dan Winship  <danw@novell.com>
 
 	* libsoup/soup-session-sync.c (send_message): signal the
diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c
index 2d736c74..7601d924 100644
--- a/libsoup/soup-connection.c
+++ b/libsoup/soup-connection.c
@@ -673,6 +673,14 @@ send_request (SoupConnection *conn, SoupMessage *req)
 				   conn->priv->proxy_uri != NULL);
 }
 
+/**
+ * soup_connection_send_request:
+ * @conn: a #SoupConnection
+ * @req: a #SoupMessage
+ *
+ * Sends @req on @conn. This is a low-level function, intended for use
+ * by #SoupSession.
+ **/
 void
 soup_connection_send_request (SoupConnection *conn, SoupMessage *req)
 {
@@ -683,6 +691,15 @@ soup_connection_send_request (SoupConnection *conn, SoupMessage *req)
 	SOUP_CONNECTION_GET_CLASS (conn)->send_request (conn, req);
 }
 
+/**
+ * soup_connection_reserve:
+ * @conn: a #SoupConnection
+ *
+ * Marks @conn as "in use" despite not actually having a message on
+ * it. This is used by #SoupSession to keep it from accidentally
+ * trying to queue two messages on the same connection from different
+ * threads at the same time.
+ **/
 void
 soup_connection_reserve (SoupConnection *conn)
 {
@@ -691,6 +708,36 @@ soup_connection_reserve (SoupConnection *conn)
 	conn->priv->in_use = TRUE;
 }
 
+/**
+ * soup_connection_release:
+ * @conn: a #SoupConnection
+ *
+ * Marks @conn as not "in use". This can be used to cancel the effect
+ * of a soup_session_reserve(). It is not necessary to call this
+ * after soup_connection_send_request().
+ **/
+void
+soup_connection_release (SoupConnection *conn)
+{
+	g_return_if_fail (SOUP_IS_CONNECTION (conn));
+
+	conn->priv->in_use = FALSE;
+}
+
+/**
+ * soup_connection_authenticate:
+ * @conn: a #SoupConnection
+ * @msg: the message to authenticate
+ * @auth_type: type of authentication to use
+ * @auth_realm: authentication realm
+ * @username: on successful return, will contain the username to
+ * authenticate with
+ * @password: on successful return, will contain the password to
+ * authenticate with
+ *
+ * Emits the %authenticate signal on @conn. For use by #SoupConnection
+ * subclasses.
+ **/
 void
 soup_connection_authenticate (SoupConnection *conn, SoupMessage *msg,
 			      const char *auth_type, const char *auth_realm,
@@ -700,6 +747,20 @@ soup_connection_authenticate (SoupConnection *conn, SoupMessage *msg,
 		       msg, auth_type, auth_realm, username, password);
 }
 
+/**
+ * soup_connection_authenticate:
+ * @conn: a #SoupConnection
+ * @msg: the message to authenticate
+ * @auth_type: type of authentication to use
+ * @auth_realm: authentication realm
+ * @username: on successful return, will contain the username to
+ * authenticate with
+ * @password: on successful return, will contain the password to
+ * authenticate with
+ *
+ * Emits the %reauthenticate signal on @conn. For use by
+ * #SoupConnection subclasses.
+ **/
 void
 soup_connection_reauthenticate (SoupConnection *conn, SoupMessage *msg,
 				const char *auth_type, const char *auth_realm,
diff --git a/libsoup/soup-connection.h b/libsoup/soup-connection.h
index 5d08b080..92583b4a 100644
--- a/libsoup/soup-connection.h
+++ b/libsoup/soup-connection.h
@@ -70,7 +70,9 @@ time_t          soup_connection_last_used      (SoupConnection   *conn);
 
 void            soup_connection_send_request   (SoupConnection   *conn,
 						SoupMessage      *req);
+
 void            soup_connection_reserve        (SoupConnection   *conn);
+void            soup_connection_release        (SoupConnection   *conn);
 
 /* protected */
 void            soup_connection_authenticate   (SoupConnection   *conn,
diff --git a/libsoup/soup-session-async.c b/libsoup/soup-session-async.c
index f6addaf9..81a9d44e 100644
--- a/libsoup/soup-session-async.c
+++ b/libsoup/soup-session-async.c
@@ -94,16 +94,33 @@ got_connection (SoupConnection *conn, guint status, gpointer user_data)
 {
 	SoupSessionAsync *sa = user_data;
 
-	if (status == SOUP_STATUS_OK) {
-		g_signal_connect (conn, "disconnected",
-				  G_CALLBACK (connection_closed),
-				  sa);
+	if (status != SOUP_STATUS_OK) {
+		/* The connection attempt failed, and thus @conn was
+		 * closed and the open connection count for the
+		 * session has been decremented. (If the failure was
+		 * fatal, then SoupSession itself will have dealt
+		 * with cancelling any pending messages for that
+		 * host, so we don't need to worry about that here.)
+		 * However, there may be other messages in the
+		 * queue that were waiting for the connection count
+		 * to go down, so run the queue now.
+		 */
+		run_queue (sa, FALSE);
+		return;
 	}
 
-	/* Either we just got a connection, or we just failed to
-	 * open a connection and so decremented the open connection
-	 * count by one. Either way, we need to run the queue now.
+	g_signal_connect (conn, "disconnected",
+			  G_CALLBACK (connection_closed), sa);
+
+	/* @conn has been marked reserved by SoupSession, but we don't
+	 * actually have any specific message in mind for it. (In
+	 * particular, the message we were originally planning to
+	 * queue on it may have already been queued on some other
+	 * connection that became available while we were waiting for
+	 * this one to connect.) So we release the connection into the
+	 * idle pool and then just run the queue and see what happens.
 	 */
+	soup_connection_release (conn);
 	run_queue (sa, FALSE);
 }
 
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index ded3cde8..78511564 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -932,6 +932,7 @@ connect_result (SoupConnection *conn, guint status, gpointer user_data)
 	}
 
 	if (status == SOUP_STATUS_OK) {
+		soup_connection_reserve (conn);
 		host->connections = g_slist_prepend (host->connections, conn);
 		g_mutex_unlock (session->priv->host_lock);
 		return;
@@ -980,20 +981,32 @@ connect_result (SoupConnection *conn, guint status, gpointer user_data)
  * @is_new: on return, %TRUE if the returned connection is new and not
  * yet connected
  * 
- * Tries to find or create a connection for @msg. If there is an idle
- * connection to the relevant host available, then it will be returned
- * (with *@is_new set to %FALSE). Otherwise, if it is possible to
- * create a new connection, one will be created and returned, with
- * *@is_new set to %TRUE.
+ * Tries to find or create a connection for @msg.
  *
- * If no connection can be made, it will return %NULL. If @session has
+ * If there is an idle connection to the relevant host available, then
+ * that connection will be returned (with *@is_new set to %FALSE). The
+ * connection will be marked "reserved", so the caller must call
+ * soup_connection_release() if it ends up not using the connection
+ * right away.
+ *
+ * If there is no idle connection available, but it is possible to
+ * create a new connection, then one will be created and returned,
+ * with *@is_new set to %TRUE. The caller MUST then call
+ * soup_connection_connect_sync() or soup_connection_connect_async()
+ * to connect it. If the connection attempt succeeds, the connection
+ * will be marked "reserved" and added to @session's connection pool
+ * once it connects. If the connection attempt fails, the connection
+ * will be unreffed.
+ *
+ * If no connection is available and a new connection cannot be made,
+ * soup_session_get_connection() will return %NULL. If @session has
  * the maximum number of open connections open, but does not have the
- * maximum number of per-host connections open to the relevant host, then
- * *@try_pruning will be set to %TRUE. In this case, the caller can
- * call soup_session_try_prune_connection() to close an idle connection,
- * and then try soup_session_get_connection() again. (If calling
- * soup_session_try_prune_connection() wouldn't help, then *@try_pruning
- * is left untouched; it is NOT set to %FALSE.)
+ * maximum number of per-host connections open to the relevant host,
+ * then *@try_pruning will be set to %TRUE. In this case, the caller
+ * can call soup_session_try_prune_connection() to close an idle
+ * connection, and then try soup_session_get_connection() again. (If
+ * calling soup_session_try_prune_connection() wouldn't help, then
+ * *@try_pruning is left untouched; it is NOT set to %FALSE.)
  *
  * Return value: a #SoupConnection, or %NULL
  **/