From 3076f99d1aa8e8c87ea6b22c0f0fdbc382e449b6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 12 Oct 2009 12:36:09 -0400 Subject: [PATCH] Don't leak session if message is cancelled during socket connection Fix up refcounting handling so that if the session is destroyed while a connection attempt is pending, it doesn't cause the session to be leaked. https://bugzilla.gnome.org/show_bug.cgi?id=596074 --- libsoup/soup-session-async.c | 78 +++++++++++++++++++++++--------------------- tests/misc-test.c | 56 +++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 38 deletions(-) diff --git a/libsoup/soup-session-async.c b/libsoup/soup-session-async.c index 89af696..9416a7a 100644 --- a/libsoup/soup-session-async.c +++ b/libsoup/soup-session-async.c @@ -245,7 +245,6 @@ tunnel_connected (SoupMessage *msg, gpointer user_data) do_idle_run_queue (data->session); done: - g_object_unref (data->session); soup_message_queue_item_unref (data->item); g_slice_free (SoupSessionAsyncTunnelData, data); } @@ -255,47 +254,50 @@ got_connection (SoupConnection *conn, guint status, gpointer session) { SoupAddress *tunnel_addr; - if (status == SOUP_STATUS_OK) { - tunnel_addr = soup_connection_get_tunnel_addr (conn); - if (tunnel_addr) { - SoupSessionAsyncTunnelData *data; - - data = g_slice_new (SoupSessionAsyncTunnelData); - data->session = session; - data->conn = conn; - data->item = soup_session_make_connect_message (session, tunnel_addr); - g_signal_emit_by_name (session, "tunneling", conn); - g_signal_connect (data->item->msg, "finished", - G_CALLBACK (tunnel_connected), data); - g_signal_connect (data->item->msg, "restarted", - G_CALLBACK (tunnel_connected), data); - soup_session_send_queue_item (session, data->item, conn); - return; - } - - g_signal_connect (conn, "disconnected", - G_CALLBACK (connection_closed), session); - - /* @conn has been marked reserved by SoupSession, but - * we don't actually have any specific message in mind - * for it. (In particular, the message we were - * originally planning to queue on it may have already - * been queued on some other connection that became - * available while we were waiting for this one to - * connect.) So we release the connection into the - * idle pool and then just run the queue and see what - * happens. + if (status != SOUP_STATUS_OK) { + /* There may have been messages waiting for the + * connection count to go down, so queue a run_queue. */ - soup_connection_set_state (conn, SOUP_CONNECTION_IDLE); - } else + do_idle_run_queue (session); + soup_session_connection_failed (session, conn, status); + /* session may be destroyed at this point */ + + return; + } - /* Even if the connection failed, we run the queue, since - * there may have been messages waiting for the connection - * count to go down. + tunnel_addr = soup_connection_get_tunnel_addr (conn); + if (tunnel_addr) { + SoupSessionAsyncTunnelData *data; + + data = g_slice_new (SoupSessionAsyncTunnelData); + data->session = session; + data->conn = conn; + data->item = soup_session_make_connect_message (session, tunnel_addr); + g_signal_emit_by_name (session, "tunneling", conn); + g_signal_connect (data->item->msg, "finished", + G_CALLBACK (tunnel_connected), data); + g_signal_connect (data->item->msg, "restarted", + G_CALLBACK (tunnel_connected), data); + soup_session_send_queue_item (session, data->item, conn); + return; + } + + g_signal_connect (conn, "disconnected", + G_CALLBACK (connection_closed), session); + + /* @conn has been marked reserved by SoupSession, but + * we don't actually have any specific message in mind + * for it. (In particular, the message we were + * originally planning to queue on it may have already + * been queued on some other connection that became + * available while we were waiting for this one to + * connect.) So we release the connection into the + * idle pool and then just run the queue and see what + * happens. */ + soup_connection_set_state (conn, SOUP_CONNECTION_IDLE); do_idle_run_queue (session); - g_object_unref (session); } static void @@ -340,7 +342,7 @@ run_queue (SoupSessionAsync *sa) if (soup_connection_get_state (conn) == SOUP_CONNECTION_NEW) { soup_connection_connect_async (conn, got_connection, - g_object_ref (session)); + session); } else soup_session_send_queue_item (session, item, conn); } diff --git a/tests/misc-test.c b/tests/misc-test.c index 70c213d..fdacbb3 100644 --- a/tests/misc-test.c +++ b/tests/misc-test.c @@ -377,6 +377,61 @@ do_star_test (void) soup_uri_free (star_uri); } +static gboolean +ea_abort_session (gpointer session) +{ + soup_session_abort (session); + return FALSE; +} + +static void +ea_connection_state_changed (GObject *conn, GParamSpec *pspec, gpointer session) +{ + SoupConnectionState state; + + g_object_get (conn, "state", &state, NULL); + if (state == SOUP_CONNECTION_CONNECTING) { + g_idle_add_full (G_PRIORITY_HIGH, + ea_abort_session, + session, NULL); + g_signal_handlers_disconnect_by_func (conn, ea_connection_state_changed, session); + } +} + +static void +ea_connection_created (SoupSession *session, GObject *conn, gpointer user_data) +{ + g_signal_connect (conn, "notify::state", + G_CALLBACK (ea_connection_state_changed), session); + g_signal_handlers_disconnect_by_func (session, ea_connection_created, user_data); +} + +static void +do_early_abort_test (void) +{ + SoupSession *session; + SoupMessage *msg; + + debug_printf (1, "\nAbort with pending connection\n"); + + session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL); + + msg = soup_message_new_from_uri ("GET", base_uri); + + g_signal_connect (session, "connection-created", + G_CALLBACK (ea_connection_created), NULL); + soup_session_send_message (session, msg); + + if (msg->status_code != SOUP_STATUS_CANCELLED) { + debug_printf (1, " Unexpected response: %d %s\n", + msg->status_code, msg->reason_phrase); + errors++; + } + g_object_unref (msg); + + soup_test_session_abort_unref (session); +} + int main (int argc, char **argv) { @@ -401,6 +456,7 @@ main (int argc, char **argv) do_callback_unref_test (); do_msg_reuse_test (); do_star_test (); + do_early_abort_test (); soup_uri_free (base_uri); -- 2.7.4