From: Dan Winship <danw@gnome.org>
Date: Wed, 10 Nov 2010 21:48:56 +0000 (-0500)
Subject: soup-message-io: fix retry-after-unexpected-connection-close
X-Git-Tag: LIBSOUP_2_32_1~5
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d4cc1a0cdd62fc3669f4ec5a8974c61aee9e9ce2;p=platform%2Fupstream%2Flibsoup.git

soup-message-io: fix retry-after-unexpected-connection-close

When sending a request on a previously-used connection, we have to
deal with the possibility of the server deciding to time out the
connection right as we start sending data (which sounds like a crazy
race condition, but is in fact pretty much standard behavior). This
got broken in the connection/session reorg earlier in the year. Fix
it.

Also, add a test to misc-test for this.

Based on patch from Sergio Villar.

https://bugzilla.gnome.org/show_bug.cgi?id=631525
---

diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c
index 815bc0b..525ea71 100644
--- a/libsoup/soup-message-io.c
+++ b/libsoup/soup-message-io.c
@@ -196,7 +196,7 @@ io_error (SoupSocket *sock, SoupMessage *msg, GError *error)
 		   !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 */
-		io->item->state = SOUP_MESSAGE_STARTING;
+		io->item->state = SOUP_MESSAGE_RESTARTING;
 	} else if (!SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code))
 		soup_message_set_status (msg, SOUP_STATUS_IO_ERROR);
 
diff --git a/tests/misc-test.c b/tests/misc-test.c
index b1c9eab..33b7bec 100644
--- a/tests/misc-test.c
+++ b/tests/misc-test.c
@@ -43,6 +43,12 @@ close_socket (SoupMessage *msg, gpointer user_data)
 }
 
 static void
+timeout_socket (SoupSocket *sock, gpointer user_data)
+{
+	soup_socket_disconnect (sock);
+}
+
+static void
 server_callback (SoupServer *server, SoupMessage *msg,
 		 const char *path, GHashTable *query,
 		 SoupClientContext *context, gpointer data)
@@ -110,6 +116,17 @@ server_callback (SoupServer *server, SoupMessage *msg,
 		return;
 	}
 
+	if (!strcmp (path, "/timeout-persistent")) {
+		SoupSocket *sock;
+
+		/* We time out the persistent connection as soon as
+		 * the client starts writing the next request.
+		 */
+		sock = soup_client_context_get_socket (context);
+		g_signal_connect (sock, "readable",
+				  G_CALLBACK (timeout_socket), NULL);
+	}
+
 	soup_message_set_status (msg, SOUP_STATUS_OK);
 	if (!strcmp (uri->host, "foo")) {
 		soup_message_set_response (msg, "text/plain",
@@ -634,6 +651,106 @@ do_accept_language_test (void)
 		g_unsetenv ("LANGUAGE");
 }
 
+static void
+timeout_test_request_started (SoupSession *session, SoupMessage *msg,
+			      SoupSocket *socket, gpointer user_data)
+{
+	SoupSocket **sockets = user_data;
+	int i;
+
+	debug_printf (2, "      msg %p => socket %p\n", msg, socket);
+	for (i = 0; i < 4; i++) {
+		if (!sockets[i]) {
+			/* We ref the socket to make sure that even if
+			 * it gets disconnected, it doesn't get freed,
+			 * since our checks would get messed up if the
+			 * slice allocator reused the same address for
+			 * two consecutive sockets.
+			 */
+			sockets[i] = g_object_ref (socket);
+			return;
+		}
+	}
+
+	debug_printf (1, "      socket queue overflowed!\n");
+	errors++;
+	soup_session_cancel_message (session, msg, SOUP_STATUS_CANCELLED);
+}
+
+static void
+do_timeout_test_for_session (SoupSession *session)
+{
+	SoupMessage *msg;
+	SoupSocket *sockets[4] = { NULL, NULL, NULL, NULL };
+	SoupURI *timeout_uri;
+	int i;
+
+	g_signal_connect (session, "request-started",
+			  G_CALLBACK (timeout_test_request_started),
+			  &sockets);
+
+	debug_printf (1, "    First message\n");
+	timeout_uri = soup_uri_new_with_base (base_uri, "/timeout-persistent");
+	msg = soup_message_new_from_uri ("GET", timeout_uri);
+	soup_uri_free (timeout_uri);
+	soup_session_send_message (session, msg);
+	if (msg->status_code != SOUP_STATUS_OK) {
+		debug_printf (1, "      Unexpected response: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	if (sockets[1]) {
+		debug_printf (1, "      Message was retried??\n");
+		errors++;
+		sockets[1] = sockets[2] = sockets[3] = NULL;
+	}
+	g_object_unref (msg);
+
+	debug_printf (1, "    Second message\n");
+	msg = soup_message_new_from_uri ("GET", base_uri);
+	soup_session_send_message (session, msg);
+	if (msg->status_code != SOUP_STATUS_OK) {
+		debug_printf (1, "      Unexpected response: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	if (sockets[1] != sockets[0]) {
+		debug_printf (1, "      Message was not retried on existing connection\n");
+		errors++;
+	} else if (!sockets[2]) {
+		debug_printf (1, "      Message was not retried after disconnect\n");
+		errors++;
+	} else if (sockets[2] == sockets[1]) {
+		debug_printf (1, "      Message was retried on closed connection??\n");
+		errors++;
+	} else if (sockets[3]) {
+		debug_printf (1, "      Message was retried again??\n");
+		errors++;
+	}
+	g_object_unref (msg);
+
+	for (i = 0; sockets[i]; i++)
+		g_object_unref (sockets[i]);
+}
+
+static void
+do_persistent_connection_timeout_test (void)
+{
+	SoupSession *session;
+
+	debug_printf (1, "\nUnexpected timing out of persistent connections\n");
+
+	debug_printf (1, "  Async session\n");
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	do_timeout_test_for_session (session);
+	soup_test_session_abort_unref (session);
+
+	debug_printf (1, "  Sync session\n");
+	session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC, NULL);
+	do_timeout_test_for_session (session);
+	soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -661,6 +778,7 @@ main (int argc, char **argv)
 	do_early_abort_test ();
 	do_content_length_framing_test ();
 	do_accept_language_test ();
+	do_persistent_connection_timeout_test ();
 
 	soup_uri_free (base_uri);
 	soup_test_server_quit_unref (server);