store the GSource in priv, don't ref the session. Otherwise the session

	* libsoup/soup-session-async.c (do_idle_run_queue): store the
	GSource in priv, don't ref the session. Otherwise the session
	won't get destroyed if you abort it and then don't return to its
	main loop. (addendum to #498509, Arnout Vandecappelle)
	(finalize): Destroy the idle_run_queue source when finalizing.
	(run_queue, got_connection): Ref the session when calling
	soup_connection_connect_async(), and do a
	do_idle_run_queue()+unref in got_connection, to ensure correct
	handling regardless of what the application does with its own ref
	on the session.
	(final_finished): Likewise, ref/do_idle_run_queue/unref rather
	than calling run_queue directly and playing with weak pointers.

	* libsoup/soup-session.c (connect_result): ref the session around
	the cancel-if-error loop

	Fixes #533473, crash in seahorse when connecting to a
	non-responsive key server.

	* tests/misc-test.c (do_callback_unref_test): Add a test for the
	bug in #533473.

	* tests/test-utils.c (soup_test_session_abort_unref): abort and
	unref a SoupSession, and consider it an error if the session still
	exists afterward. Suggested by Arnout Vandecappelle.
	(test_server_shutdown): Likewise, consider it an error if the
	server is leaked.

	* tests/*.c: Use soup_test_session_abort_unref().

svn path=/trunk/; revision=1168

11 files changed, 146 insertions, 49 deletions

diff --git a/tests/auth-test.c b/tests/auth-test.c
index 08dfda3f..96a4979d 100644
--- a/tests/auth-test.c
+++ b/tests/auth-test.c
@@ -503,8 +503,7 @@ do_async_auth_test (const char *base_uri)
 		errors++;
 	}
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	g_object_unref (msg1);
 	g_object_unref (msg3);
@@ -537,8 +536,7 @@ do_async_auth_test (const char *base_uri)
 	g_main_loop_run (loop);
 	g_signal_handler_disconnect (session, auth_id);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	g_object_unref (msg1);
 
@@ -607,8 +605,7 @@ main (int argc, char **argv)
 
 		g_object_unref (msg);
 	}
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	/* And now for some regression tests */
 	loop = g_main_loop_new (NULL, TRUE);
@@ -633,8 +630,7 @@ main (int argc, char **argv)
 	g_free (uri);
 
 	g_main_loop_run (loop);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	debug_printf (1, "\nTesting digest nonce expiration:\n");
 
@@ -726,8 +722,7 @@ main (int argc, char **argv)
 	do_digest_nonce_test (session, "Fourth", uri, FALSE, FALSE);
 	g_free (uri);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	/* Async auth */
 	do_async_auth_test (base_uri);
diff --git a/tests/chunk-test.c b/tests/chunk-test.c
index e79dc93f..13238cf2 100644
--- a/tests/chunk-test.c
+++ b/tests/chunk-test.c
@@ -248,8 +248,7 @@ do_chunk_tests (SoupURI *base_uri)
 	do_request_test (session, base_uri);
 	debug_printf (2, "\n\n");
 	do_response_test (session, base_uri);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 }
 
 static void
diff --git a/tests/context-test.c b/tests/context-test.c
index c2260519..6cd9ec7c 100644
--- a/tests/context-test.c
+++ b/tests/context-test.c
@@ -194,8 +194,7 @@ test1_thread (gpointer user_data)
 	}
 	g_object_unref (msg);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 	g_free (uri);
 
 	g_cond_signal (test1_cond);
@@ -240,8 +239,7 @@ do_test2 (void)
 	}
 	g_object_unref (msg);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 	g_free (uri);
 
 	g_source_remove (idle);
diff --git a/tests/continue-test.c b/tests/continue-test.c
index 5fbcbe37..d0905cef 100644
--- a/tests/continue-test.c
+++ b/tests/continue-test.c
@@ -109,8 +109,7 @@ do_message (const char *path, gboolean long_body,
 	events = NULL;
 	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
 	soup_session_send_message (session, msg);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	va_start (ap, auth);
 	while ((expected_event = va_arg (ap, const char *))) {
diff --git a/tests/misc-test.c b/tests/misc-test.c
index 765d70b6..d5985b82 100644
--- a/tests/misc-test.c
+++ b/tests/misc-test.c
@@ -46,8 +46,8 @@ server_callback (SoupServer *server, SoupMessage *msg,
 
 /* Host header handling: client must be able to override the default
  * value, server must be able to recognize different Host values.
+ * #539803.
  */
-
 static void
 do_host_test (void)
 {
@@ -65,8 +65,7 @@ do_host_test (void)
 	soup_session_send_message (session, one);
 	soup_session_send_message (session, two);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	if (!SOUP_STATUS_IS_SUCCESSFUL (one->status_code)) {
 		debug_printf (1, "  Message 1 failed: %d %s\n",
@@ -91,6 +90,96 @@ do_host_test (void)
 	g_object_unref (two);
 }
 
+/* Dropping the application's ref on the session from a callback
+ * should not cause the session to be freed at an incorrect time.
+ * (This test will crash if it fails.) #533473
+ */
+static void
+cu_one_completed (SoupSession *session, SoupMessage *msg, gpointer loop)
+{
+	debug_printf (2, "  Message 1 completed\n");
+	if (msg->status_code != SOUP_STATUS_CANT_CONNECT) {
+		debug_printf (1, "  Unexpected status on Message 1: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	g_object_unref (session);
+}
+
+static gboolean
+cu_idle_quit (gpointer loop)
+{
+	g_main_loop_quit (loop);
+	return FALSE;
+}
+
+static void
+cu_two_completed (SoupSession *session, SoupMessage *msg, gpointer loop)
+{
+	debug_printf (2, "  Message 2 completed\n");
+	if (msg->status_code != SOUP_STATUS_CANT_CONNECT) {
+		debug_printf (1, "  Unexpected status on Message 2: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+	g_idle_add (cu_idle_quit, loop); 
+}
+
+static void
+do_callback_unref_test (void)
+{
+	SoupServer *bad_server;
+	SoupSession *session;
+	SoupMessage *one, *two;
+	GMainLoop *loop;
+	char *bad_uri;
+
+	debug_printf (1, "Callback unref handling\n");
+
+	/* Get a guaranteed-bad URI */
+	bad_server = soup_server_new (NULL, NULL);
+	bad_uri = g_strdup_printf ("http://localhost:%u/",
+				   soup_server_get_port (bad_server));
+	g_object_unref (bad_server);
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	g_object_add_weak_pointer (G_OBJECT (session), (gpointer *)&session);
+
+	loop = g_main_loop_new (NULL, TRUE);
+
+	one = soup_message_new ("GET", bad_uri);
+	g_object_add_weak_pointer (G_OBJECT (one), (gpointer *)&one);
+	two = soup_message_new ("GET", bad_uri);
+	g_object_add_weak_pointer (G_OBJECT (two), (gpointer *)&two);
+
+	soup_session_queue_message (session, one, cu_one_completed, loop);
+	soup_session_queue_message (session, two, cu_two_completed, loop);
+
+	g_main_loop_run (loop);
+	g_main_loop_unref (loop);
+
+	if (session) {
+		g_object_remove_weak_pointer (G_OBJECT (session), (gpointer *)&session);
+		debug_printf (1, "  Session not destroyed?\n");
+		errors++;
+		g_object_unref (session);
+	}
+	if (one) {
+		g_object_remove_weak_pointer (G_OBJECT (one), (gpointer *)&one);
+		debug_printf (1, "  Message 1 not destroyed?\n");
+		errors++;
+		g_object_unref (one);
+	}
+	if (two) {
+		g_object_remove_weak_pointer (G_OBJECT (two), (gpointer *)&two);
+		debug_printf (1, "  Message 2 not destroyed?\n");
+		errors++;
+		g_object_unref (two);
+	}
+
+	/* Otherwise, if we haven't crashed, we're ok. */
+}
+
 int
 main (int argc, char **argv)
 {
@@ -104,6 +193,7 @@ main (int argc, char **argv)
 				    soup_server_get_port (server));
 
 	do_host_test ();
+	do_callback_unref_test ();
 
 	g_free (base_uri);
 
diff --git a/tests/ntlm-test.c b/tests/ntlm-test.c
index fc4c0823..6c6d6d37 100644
--- a/tests/ntlm-test.c
+++ b/tests/ntlm-test.c
@@ -385,8 +385,7 @@ do_ntlm_round (SoupURI *base_uri, gboolean use_ntlm, const char *user)
 		    user != NULL ? SOUP_STATUS_OK :
 		    SOUP_STATUS_UNAUTHORIZED);
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 }
 
 static void
diff --git a/tests/pull-api.c b/tests/pull-api.c
index 906a5e91..d47ce91b 100644
--- a/tests/pull-api.c
+++ b/tests/pull-api.c
@@ -41,8 +41,7 @@ get_correct_response (const char *uri)
 	correct_response = soup_message_body_flatten (msg->response_body);
 
 	g_object_unref (msg);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 }
 
 /* Pull API version 1: fully-async. More like a "poke" API. Rather
@@ -508,8 +507,7 @@ main (int argc, char **argv)
 			     TRUE, SOUP_STATUS_UNAUTHORIZED);
 	do_fully_async_test (session, base_uri, "/Basic/realm2/",
 			     TRUE, SOUP_STATUS_OK);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	debug_printf (1, "\nFully async, slow requests\n");
 	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
@@ -521,8 +519,7 @@ main (int argc, char **argv)
 			     FALSE, SOUP_STATUS_UNAUTHORIZED);
 	do_fully_async_test (session, base_uri, "/Basic/realm2/",
 			     FALSE, SOUP_STATUS_OK);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	debug_printf (1, "\nSynchronously async\n");
 	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
@@ -534,9 +531,7 @@ main (int argc, char **argv)
 				     SOUP_STATUS_UNAUTHORIZED);
 	do_synchronously_async_test (session, base_uri, "/Basic/realm2/",
 				     SOUP_STATUS_OK);
-
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	soup_buffer_free (correct_response);
 
diff --git a/tests/redirect-test.c b/tests/redirect-test.c
index 048104e0..5bb37e95 100644
--- a/tests/redirect-test.c
+++ b/tests/redirect-test.c
@@ -189,15 +189,13 @@ do_redirect_tests (SoupURI *base_uri)
 	debug_printf (1, "Async session\n");
 	for (n = 0; n < n_tests; n++)
 		do_test (session, base_uri, n);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC, NULL);
 	debug_printf (1, "Sync session\n");
 	for (n = 0; n < n_tests; n++)
 		do_test (session, base_uri, n);
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 }
 
 static void
diff --git a/tests/test-utils.c b/tests/test-utils.c
index f0f2860d..0bb4a89a 100644
--- a/tests/test-utils.c
+++ b/tests/test-utils.c
@@ -100,14 +100,6 @@ test_init (int argc, char **argv, GOptionEntry *entries)
 void
 test_cleanup (void)
 {
-	debug_printf (1, "\n");
-	if (errors) {
-		printf ("%s: %d error(s).%s\n",
-			g_get_prgname (), errors,
-			debug_level == 0 ? " Run with '-d' for details" : "");
-	} else
-		printf ("%s: OK\n", g_get_prgname ());
-
 #ifdef HAVE_APACHE
 	if (apache_running)
 		apache_cleanup ();
@@ -119,6 +111,14 @@ test_cleanup (void)
 		g_object_unref (logger);
 
 	g_main_context_unref (g_main_context_default ());
+
+	debug_printf (1, "\n");
+	if (errors) {
+		printf ("%s: %d error(s).%s\n",
+			g_get_prgname (), errors,
+			debug_level == 0 ? " Run with '-d' for details" : "");
+	} else
+		printf ("%s: OK\n", g_get_prgname ());
 }
 
 void
@@ -225,6 +225,21 @@ soup_test_session_new (GType type, ...)
 	return session;
 }
 
+void
+soup_test_session_abort_unref (SoupSession *session)
+{
+	g_object_add_weak_pointer (G_OBJECT (session), (gpointer *)&session);
+
+	soup_session_abort (session);
+	g_object_unref (session);
+
+	if (session) {
+		errors++;
+		debug_printf (1, "leaked SoupSession!\n");
+		g_object_remove_weak_pointer (G_OBJECT (session), (gpointer *)&session);
+	}
+}
+
 static gpointer run_server_thread (gpointer user_data);
 
 SoupServer *
@@ -271,6 +286,9 @@ idle_quit_server (gpointer server)
 static void
 test_server_shutdown (void)
 {
+	g_object_add_weak_pointer (G_OBJECT (test_server),
+				   (gpointer *)&test_server);
+
 	if (server_thread) {
 		soup_add_completion (soup_server_get_async_context (test_server),
 				     idle_quit_server, test_server);
@@ -278,6 +296,11 @@ test_server_shutdown (void)
 	} else
 		soup_server_quit (test_server);
 	g_object_unref (test_server);
-}
-
 
+	if (test_server) {
+		errors++;
+		debug_printf (1, "leaked SoupServer!\n");
+		g_object_remove_weak_pointer (G_OBJECT (test_server),
+					      (gpointer *)&test_server);
+	}
+}
diff --git a/tests/test-utils.h b/tests/test-utils.h
index 535172fc..ff9f472e 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -15,5 +15,7 @@ void apache_init    (void);
 void apache_cleanup (void);
 #endif
 
-SoupSession *soup_test_session_new (GType type, ...);
+SoupSession *soup_test_session_new         (GType type, ...);
+void         soup_test_session_abort_unref (SoupSession *session);
+
 SoupServer  *soup_test_server_new  (gboolean in_own_thread);
diff --git a/tests/xmlrpc-test.c b/tests/xmlrpc-test.c
index b7735492..372a6f69 100644
--- a/tests/xmlrpc-test.c
+++ b/tests/xmlrpc-test.c
@@ -468,8 +468,7 @@ main (int argc, char **argv)
 	if (!test_fault_args ())
 		errors++;
 
-	soup_session_abort (session);
-	g_object_unref (session);
+	soup_test_session_abort_unref (session);
 
 	test_cleanup ();
 	return errors != 0;