Re-redo not-yet-used connection handling
authorDan Winship <danw@gnome.org>
Fri, 21 Aug 2009 23:03:56 +0000 (19:03 -0400)
committerDan Winship <danw@gnome.org>
Sat, 22 Aug 2009 13:32:24 +0000 (09:32 -0400)
Previously we were taking special care to avoid closing connections
that hadn't been used yet, but this is not actually necessary;
run_queue() doesn't prune idle connections until after running through
the entire queue once, so if the new connection is still idle at that
point, it means we no longer have a message to send on it (probably
because the message already got sent on another connection), and so it
is legitimate to close it if needed.

http://bugzilla.gnome.org/show_bug.cgi?id=592084

libsoup/soup-connection.c
libsoup/soup-connection.h
libsoup/soup-session.c

index dc9c0b1..c4a3846 100644 (file)
@@ -38,7 +38,7 @@ typedef struct {
 
        SoupMessage *cur_req;
        SoupConnectionState state;
-       time_t       last_used;
+       gboolean     ever_used;
        guint        io_timeout, idle_timeout;
        GSource     *idle_timeout_src;
 } SoupConnectionPrivate;
@@ -78,9 +78,7 @@ static void clear_current_request (SoupConnection *conn);
 static void
 soup_connection_init (SoupConnection *conn)
 {
-       SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn);
-
-       priv->last_used = time (NULL);
+       ;
 }
 
 static void
@@ -287,7 +285,6 @@ start_idle_timer (SoupConnection *conn)
 {
        SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn);
 
-       priv->last_used = time (NULL);
        if (priv->idle_timeout > 0 && !priv->idle_timeout_src) {
                priv->idle_timeout_src =
                        soup_add_timeout (priv->async_context,
@@ -337,8 +334,10 @@ clear_current_request (SoupConnection *conn)
 
                if (!soup_message_is_keepalive (cur_req))
                        soup_connection_disconnect (conn);
-               else
+               else {
+                       priv->ever_used = TRUE;
                        soup_message_io_stop (cur_req);
+               }
        }
 }
 
@@ -538,10 +537,12 @@ soup_connection_disconnect (SoupConnection *conn)
 
        if (priv->cur_req &&
            priv->cur_req->status_code == SOUP_STATUS_IO_ERROR &&
-           soup_connection_get_age (conn) > 5) {
+           priv->ever_used) {
                /* There was a message queued on this connection, but
                 * the socket was closed while it was being sent.
-                * Since the connection is idle, the most likely cause of
+                * Since ever_used is TRUE, then that means at least
+                * one message was successfully sent on this
+                * connection before, and so the most likely cause of
                 * the IO_ERROR is that the connection was idle for
                 * too long and the server timed out and closed it
                 * (and we didn't notice until after we started
@@ -558,14 +559,14 @@ soup_connection_disconnect (SoupConnection *conn)
                                            SOUP_MESSAGE_IO_STATUS_QUEUED);
        }
 
-       /* If cur_req is non-NULL but the connection was NOT idle when
-        * it was sent, then that means this was the first message to
-        * be sent on this connection, and it failed, so the error
-        * probably means that there's some network or server problem,
-        * so we let the IO_ERROR be returned to the caller.
+       /* If cur_req is non-NULL but priv->ever_used is FALSE, then that
+        * means this was the first message to be sent on this
+        * connection, and it failed, so the error probably means that
+        * there's some network or server problem, so we let the
+        * IO_ERROR be returned to the caller.
         *
         * (Of course, it's also possible that the error in the
-        * idle-connection case was because of a network/server problem
+        * ever_used == TRUE case was because of a network/server problem
         * too. It's even possible that the message crashed the
         * server. In this case, requeuing it was the wrong thing to
         * do, but presumably, the next attempt will also get an
@@ -630,14 +631,6 @@ soup_connection_set_state (SoupConnection *conn, SoupConnectionState state)
                clear_current_request (conn);
 }
 
-guint
-soup_connection_get_age (SoupConnection *conn)
-{
-       g_return_val_if_fail (SOUP_IS_CONNECTION (conn), FALSE);
-
-       return time (NULL) - SOUP_CONNECTION_GET_PRIVATE (conn)->last_used;
-}
-
 /**
  * soup_connection_send_request:
  * @conn: a #SoupConnection
index 6497a11..aa261d2 100644 (file)
@@ -74,7 +74,6 @@ SoupURI        *soup_connection_get_proxy_uri  (SoupConnection   *conn);
 SoupConnectionState soup_connection_get_state  (SoupConnection   *conn);
 void                soup_connection_set_state  (SoupConnection   *conn,
                                                SoupConnectionState state);
-guint           soup_connection_get_age        (SoupConnection   *conn);
 
 void            soup_connection_send_request   (SoupConnection   *conn,
                                                SoupMessage      *req);
index d841ca0..bcfb816 100644 (file)
@@ -972,8 +972,7 @@ soup_session_cleanup_connections (SoupSession *session,
        while (g_hash_table_iter_next (&iter, &conn, &host)) {
                state = soup_connection_get_state (conn);
                if (state == SOUP_CONNECTION_REMOTE_DISCONNECTED ||
-                   (prune_idle && state == SOUP_CONNECTION_IDLE &&
-                    soup_connection_get_age (conn) > 5))
+                   (prune_idle && state == SOUP_CONNECTION_IDLE))
                        conns = g_slist_prepend (conns, g_object_ref (conn));
        }
        g_mutex_unlock (priv->host_lock);