From 84fd3e9bfc19dc64f0502701ec633c99d9fd0ff9 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 23 Nov 2010 12:53:37 -0500 Subject: [PATCH] SoupSessionSync: fix messages getting stuck forever Messages in SoupSessionSync were getting stuck forever if they didn't manage to get a connection on the first try; the test programs didn't pick this up because none of them queue enough messages all at once to hit the max-conns limit. Fix the messages-getting-stuck bug, and add a test case to misc-test to verify it. Also fix another SoupSessionSync bug where cancelling several messages at once could cause spurious errors. https://bugzilla.gnome.org/show_bug.cgi?id=634422 --- libsoup/soup-session-sync.c | 3 ++ tests/misc-test.c | 126 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/libsoup/soup-session-sync.c b/libsoup/soup-session-sync.c index 408e307..344f234 100644 --- a/libsoup/soup-session-sync.c +++ b/libsoup/soup-session-sync.c @@ -298,6 +298,7 @@ process_queue_item (SoupMessageQueueItem *item) item->state = SOUP_MESSAGE_FINISHED; soup_message_finished (item->msg); soup_session_unqueue_item (session, item); + g_cond_broadcast (priv->cond); break; default: @@ -373,8 +374,10 @@ cancel_message (SoupSession *session, SoupMessage *msg, guint status_code) { SoupSessionSyncPrivate *priv = SOUP_SESSION_SYNC_GET_PRIVATE (session); + g_mutex_lock (priv->lock); SOUP_SESSION_CLASS (soup_session_sync_parent_class)->cancel_message (session, msg, status_code); g_cond_broadcast (priv->cond); + g_mutex_unlock (priv->lock); } static void diff --git a/tests/misc-test.c b/tests/misc-test.c index 71263ee..d3af982 100644 --- a/tests/misc-test.c +++ b/tests/misc-test.c @@ -20,6 +20,7 @@ SoupServer *server; SoupURI *base_uri; +GMutex *server_mutex; static gboolean auth_callback (SoupAuthDomain *auth_domain, SoupMessage *msg, @@ -102,6 +103,13 @@ server_callback (SoupServer *server, SoupMessage *msg, { SoupURI *uri = soup_message_get_uri (msg); + /* The way this gets used in the tests, we don't actually + * need to hold it through the whole function, so it's simpler + * to just release it right away. + */ + g_mutex_lock (server_mutex); + g_mutex_unlock (server_mutex); + soup_message_headers_append (msg->response_headers, "X-Handled-By", "server_callback"); @@ -794,6 +802,121 @@ do_persistent_connection_timeout_test (void) soup_test_session_abort_unref (session); } +static GMainLoop *max_conns_loop; +static int msgs_done; +#define MAX_CONNS 2 +#define TEST_CONNS (MAX_CONNS * 2) + +static gboolean +idle_start_server (gpointer data) +{ + g_mutex_unlock (server_mutex); + return FALSE; +} + +static gboolean +quit_loop (gpointer data) +{ + g_main_loop_quit (max_conns_loop); + return FALSE; +} + +static void +max_conns_request_started (SoupSession *session, SoupMessage *msg, + SoupSocket *socket, gpointer user_data) +{ + if (++msgs_done == MAX_CONNS) + g_timeout_add (100, quit_loop, NULL); +} + +static void +max_conns_message_complete (SoupSession *session, SoupMessage *msg, gpointer user_data) +{ + if (++msgs_done == TEST_CONNS) + g_main_loop_quit (max_conns_loop); +} + +static void +do_max_conns_test_for_session (SoupSession *session) +{ + SoupMessage *msgs[TEST_CONNS]; + int i; + guint timeout_id; + + max_conns_loop = g_main_loop_new (NULL, TRUE); + + g_mutex_lock (server_mutex); + + g_signal_connect (session, "request-started", + G_CALLBACK (max_conns_request_started), NULL); + msgs_done = 0; + for (i = 0; i < TEST_CONNS; i++) { + msgs[i] = soup_message_new_from_uri ("GET", base_uri); + g_object_ref (msgs[i]); + soup_session_queue_message (session, msgs[i], + max_conns_message_complete, NULL); + } + + g_main_loop_run (max_conns_loop); + if (msgs_done != MAX_CONNS) { + debug_printf (1, " Queued %d connections out of max %d?", + msgs_done, MAX_CONNS); + errors++; + } + g_signal_handlers_disconnect_by_func (session, max_conns_request_started, NULL); + + msgs_done = 0; + g_idle_add (idle_start_server, NULL); + timeout_id = g_timeout_add (1000, quit_loop, NULL); + g_main_loop_run (max_conns_loop); + + for (i = 0; i < TEST_CONNS; i++) { + if (!SOUP_STATUS_IS_SUCCESSFUL (msgs[i]->status_code)) { + debug_printf (1, " Message %d failed? %d %s\n", + i, msgs[i]->status_code, + msgs[i]->reason_phrase ? msgs[i]->reason_phrase : "-"); + errors++; + } + } + + if (msgs_done != TEST_CONNS) { + /* Clean up so we don't get a spurious "Leaked + * session" error. + */ + for (i = 0; i < TEST_CONNS; i++) + soup_session_cancel_message (session, msgs[i], SOUP_STATUS_CANCELLED); + g_main_loop_run (max_conns_loop); + g_source_remove (timeout_id); + } + + g_main_loop_unref (max_conns_loop); + + for (i = 0; i < TEST_CONNS; i++) + g_object_unref (msgs[i]); +} + +static void +do_max_conns_test (void) +{ + SoupSession *session; + + debug_printf (1, "\nExceeding max-conns\n"); + + debug_printf (1, " Async session\n"); + session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, + SOUP_SESSION_MAX_CONNS, MAX_CONNS, + NULL); + do_max_conns_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, + SOUP_SESSION_MAX_CONNS, MAX_CONNS, + NULL); + do_max_conns_test_for_session (session); + soup_test_session_abort_unref (session); +} + int main (int argc, char **argv) { @@ -801,6 +924,8 @@ main (int argc, char **argv) test_init (argc, argv, NULL); + server_mutex = g_mutex_new (); + server = soup_test_server_new (TRUE); soup_server_add_handler (server, NULL, server_callback, NULL, NULL); base_uri = soup_uri_new ("http://127.0.0.1/"); @@ -822,6 +947,7 @@ main (int argc, char **argv) do_content_length_framing_test (); do_accept_language_test (); do_persistent_connection_timeout_test (); + do_max_conns_test (); soup_uri_free (base_uri); soup_test_server_quit_unref (server); -- 2.7.4