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

diff --git a/libsoup/soup-session-async.c b/libsoup/soup-session-async.c
index 89af696..9416a7a 100644 (file)
--- 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 (file)
--- 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);