From d4cc1a0cdd62fc3669f4ec5a8974c61aee9e9ce2 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 10 Nov 2010 16:48:56 -0500 Subject: [PATCH] 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 --- libsoup/soup-message-io.c | 2 +- tests/misc-test.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) 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); -- 2.7.4