From 86b05405598f1232077ac926d81e93378b62f377 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 14 Jul 2004 15:21:07 +0000 Subject: [PATCH] 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 --- ChangeLog | 18 +++++++++++++ libsoup/soup-connection.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ libsoup/soup-connection.h | 2 ++ libsoup/soup-session-async.c | 31 +++++++++++++++++----- libsoup/soup-session.c | 37 ++++++++++++++++++--------- 5 files changed, 130 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index f902116..12c06f2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2004-07-13 Dan Winship + + * 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 * libsoup/soup-session-sync.c (send_message): signal the diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c index 2d736c7..7601d92 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 5d08b08..92583b4 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 f6addaf..81a9d44 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 ded3cde..7851156 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 **/ -- 2.7.4