[libsoup] Fix the latest round of "Connection terminated unexpectedly" errors

[Dan Winship <danw src gnome org>]

commit 897883bc4dcabe1814c5d14fb65c7a64e3b4a604
Author: Dan Winship <danw gnome org>
Date:   Sun Oct 18 11:21:36 2009 -0400

    Fix the latest round of "Connection terminated unexpectedly" errors
    
    The fix introduced in ef7fc058 for bug 578990 caught most cases where
    we'd write a request to an already-closed connection, but not all.
    Unfortunately, at some point the code path for recovering from that
    case got broken, so when it did happen, we'd always end up returning
    the error to the app rather than retrying the request. This is now
    fixed again.
    
    The specific recent cause of the problem was that the
    multiple-pending-connections fix (baa316ac/bug 594768) results in us
    sometimes opening more connections than we need, and so some of those
    connections become idle without ever having had a message sent across
    them. Servers tend to have different timeouts/behaviors here than with
    idle persistent connections, so we should time them out more
    aggressively. So now we close a connection after 3 seconds if it
    hasn't been used yet.

 libsoup/soup-connection.c |   58 ++++++++++----------------------------------
 libsoup/soup-message-io.c |   19 ++++++++++++++-
 libsoup/soup-socket.c     |   14 +++++++++-
 3 files changed, 43 insertions(+), 48 deletions(-)
---
diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c
index a98cf34..9f59816 100644
--- a/libsoup/soup-connection.c
+++ b/libsoup/soup-connection.c
@@ -38,7 +38,7 @@ typedef struct {
 
 	SoupMessage *cur_req;
 	SoupConnectionState state;
-	gboolean     ever_used;
+	time_t       unused_timeout;
 	guint        io_timeout, idle_timeout;
 	GSource     *idle_timeout_src;
 } SoupConnectionPrivate;
@@ -75,6 +75,11 @@ static void get_property (GObject *object, guint prop_id,
 static void stop_idle_timer (SoupConnectionPrivate *priv);
 static void clear_current_request (SoupConnection *conn);
 
+/* Number of seconds after which we close a connection that hasn't yet
+ * been used.
+ */
+#define SOUP_CONNECTION_UNUSED_TIMEOUT 3
+
 static void
 soup_connection_init (SoupConnection *conn)
 {
@@ -310,6 +315,7 @@ set_current_request (SoupConnectionPrivate *priv, SoupMessage *req)
 	g_return_if_fail (priv->cur_req == NULL);
 
 	stop_idle_timer (priv);
+	priv->unused_timeout = 0;
 
 	soup_message_set_io_status (req, SOUP_MESSAGE_IO_STATUS_RUNNING);
 	priv->cur_req = req;
@@ -336,10 +342,8 @@ clear_current_request (SoupConnection *conn)
 
 		if (!soup_message_is_keepalive (cur_req))
 			soup_connection_disconnect (conn);
-		else {
-			priv->ever_used = TRUE;
+		else
 			soup_message_io_stop (cur_req);
-		}
 	}
 }
 
@@ -375,6 +379,7 @@ socket_connect_result (SoupSocket *sock, guint status, gpointer user_data)
 			  G_CALLBACK (socket_disconnected), data->conn);
 
 	priv->state = SOUP_CONNECTION_IDLE;
+	priv->unused_timeout = time (NULL) + SOUP_CONNECTION_UNUSED_TIMEOUT;
 	start_idle_timer (data->conn);
 
  done:
@@ -467,6 +472,7 @@ soup_connection_connect_sync (SoupConnection *conn)
 
 	if (SOUP_STATUS_IS_SUCCESSFUL (status)) {
 		priv->state = SOUP_CONNECTION_IDLE;
+		priv->unused_timeout = time (NULL) + SOUP_CONNECTION_UNUSED_TIMEOUT;
 		start_idle_timer (conn);
 	} else {
 	fail:
@@ -536,47 +542,6 @@ soup_connection_disconnect (SoupConnection *conn)
 		return;
 
 	priv->state = SOUP_CONNECTION_DISCONNECTED;
-
-	if (priv->cur_req &&
-	    priv->cur_req->status_code == SOUP_STATUS_IO_ERROR &&
-	    priv->ever_used) {
-		/* There was a message queued on this connection, but
-		 * the socket was closed while it was being sent.
-		 * 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
-		 * sending the message). So we want the message to get
-		 * tried again on a new connection. The only code path
-		 * that could have gotten us to this point is through
-		 * the call to io_cleanup() in
-		 * soup_message_io_finished(), and so all we need to
-		 * do to get the message requeued in this case is to
-		 * change its status.
-		 */
-		soup_message_cleanup_response (priv->cur_req);
-		soup_message_set_io_status (priv->cur_req,
-					    SOUP_MESSAGE_IO_STATUS_QUEUED);
-	}
-
-	/* 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
-	 * 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
-	 * error, and eventually the message will be requeued onto a
-	 * fresh connection and get an error, at which point the error
-	 * will finally be returned to the caller.)
-	 */
-
 	/* NB: this might cause conn to be destroyed. */
 	g_signal_emit (conn, signals[DISCONNECTED], 0);
 }
@@ -617,6 +582,9 @@ soup_connection_get_state (SoupConnection *conn)
 			priv->state = SOUP_CONNECTION_REMOTE_DISCONNECTED;
 	}
 #endif
+	if (priv->state == SOUP_CONNECTION_IDLE &&
+	    priv->unused_timeout && priv->unused_timeout < time (NULL))
+		priv->state = SOUP_CONNECTION_REMOTE_DISCONNECTED;
 
 	return priv->state;
 }
diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c
index d88a0a4..aae4e28 100644
--- a/libsoup/soup-message-io.c
+++ b/libsoup/soup-message-io.c
@@ -179,10 +179,27 @@ soup_message_io_finished (SoupMessage *msg)
 
 static void io_read (SoupSocket *sock, SoupMessage *msg);
 
+static gboolean
+request_is_idempotent (SoupMessage *msg)
+{
+	/* FIXME */
+	return (msg->method == SOUP_METHOD_GET);
+}
+
 static void
 io_error (SoupSocket *sock, SoupMessage *msg, GError *error)
 {
-	if (!SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) {
+	SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg);
+	SoupMessageIOData *io = priv->io_data;
+
+	if (io->mode == SOUP_MESSAGE_IO_CLIENT &&
+	    io->read_state <= SOUP_MESSAGE_IO_STATE_HEADERS &&
+	    io->read_meta_buf->len == 0 &&
+	    !g_error_matches (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT) &&
+	    request_is_idempotent (msg)) {
+		/* Connection got closed, but we can safely try again */
+		priv->io_status = SOUP_MESSAGE_IO_STATUS_QUEUED;
+	} else if (!SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) {
 		if (error && error->domain == SOUP_SSL_ERROR) {
 			soup_message_set_status_full (msg,
 						      SOUP_STATUS_SSL_FAILED,
diff --git a/libsoup/soup-socket.c b/libsoup/soup-socket.c
index 97b269a..51bbe5e 100644
--- a/libsoup/soup-socket.c
+++ b/libsoup/soup-socket.c
@@ -1206,8 +1206,11 @@ read_from_network (SoupSocket *sock, gpointer buffer, gsize len,
 	if (!priv->iochannel)
 		return SOUP_SOCKET_EOF;
 
-	if (priv->timed_out)
+	if (priv->timed_out) {
+		g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
+				     "Timed out");
 		return SOUP_SOCKET_ERROR;
+	}
 
 	status = g_io_channel_read_chars (priv->iochannel,
 					  buffer, len, nread, &my_err);
@@ -1230,8 +1233,11 @@ read_from_network (SoupSocket *sock, gpointer buffer, gsize len,
 		 * a socket timeout and should be treated as an error
 		 * condition.
 		 */
-		if (!priv->non_blocking)
+		if (!priv->non_blocking) {
+			g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
+					     "Timed out");
 			return SOUP_SOCKET_ERROR;
+		}
 
 		if (!priv->read_src) {
 			priv->read_src =
@@ -1496,6 +1502,8 @@ soup_socket_write (SoupSocket *sock, gconstpointer buffer,
 	}
 	if (priv->timed_out) {
 		g_mutex_unlock (priv->iolock);
+		g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
+				     "Timed out");
 		return SOUP_SOCKET_ERROR;
 	}
 	if (priv->write_src) {
@@ -1517,6 +1525,8 @@ soup_socket_write (SoupSocket *sock, gconstpointer buffer,
 	 */
 	if (!priv->non_blocking && status == G_IO_STATUS_AGAIN) {
 		g_mutex_unlock (priv->iolock);
+		g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
+				     "Timed out");
 		return SOUP_SOCKET_ERROR;
 	}