+2008-09-30 Dan Winship <danw@gnome.org>
+
+ * 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().
+
2008-09-26 Dan Winship <danw@gnome.org>
* libsoup/soup-auth-manager-ntlm.c
**/
static gboolean run_queue (SoupSessionAsync *sa);
-static void do_idle_run_queue (SoupSession *session);
+static void do_idle_run_queue (SoupSessionAsync *sa);
static void queue_message (SoupSession *session, SoupMessage *req,
SoupSessionCallback callback, gpointer user_data);
G_DEFINE_TYPE (SoupSessionAsync, soup_session_async, SOUP_TYPE_SESSION)
+typedef struct {
+ GSource *idle_run_queue_source;
+} SoupSessionAsyncPrivate;
+#define SOUP_SESSION_ASYNC_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), SOUP_TYPE_SESSION_ASYNC, SoupSessionAsyncPrivate))
+
static void
soup_session_async_init (SoupSessionAsync *sa)
{
}
static void
+finalize (GObject *object)
+{
+ SoupSessionAsyncPrivate *priv = SOUP_SESSION_ASYNC_GET_PRIVATE (object);
+
+ if (priv->idle_run_queue_source)
+ g_source_destroy (priv->idle_run_queue_source);
+
+ G_OBJECT_CLASS (soup_session_async_parent_class)->finalize (object);
+}
+
+static void
soup_session_async_class_init (SoupSessionAsyncClass *soup_session_async_class)
{
SoupSessionClass *session_class = SOUP_SESSION_CLASS (soup_session_async_class);
+ GObjectClass *object_class = G_OBJECT_CLASS (session_class);
+
+ g_type_class_add_private (soup_session_async_class,
+ sizeof (SoupSessionAsyncPrivate));
/* virtual method override */
session_class->queue_message = queue_message;
session_class->send_message = send_message;
+
+ object_class->finalize = finalize;
}
static void
-connection_closed (SoupConnection *conn, gpointer session)
+connection_closed (SoupConnection *conn, gpointer sa)
{
/* Run the queue in case anyone was waiting for a connection
* to be closed.
*/
- do_idle_run_queue (session);
+ do_idle_run_queue (sa);
}
static void
{
SoupSessionAsync *sa = user_data;
- if (status != SOUP_STATUS_OK) {
- /* The connection attempt failed, and thus @conn was
- * closed and the open connection count for the
- * session has been decremented. (If the failure was
- * fatal, then SoupSession itself will have dealt
- * with cancelling any pending messages for that
- * host, so we don't need to worry about that here.)
- * However, there may be other messages in the
- * queue that were waiting for the connection count
- * to go down, so run the queue now.
+ if (status == SOUP_STATUS_OK) {
+ g_signal_connect (conn, "disconnected",
+ G_CALLBACK (connection_closed), sa);
+
+ /* @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.
*/
- run_queue (sa);
- return;
+ soup_connection_release (conn);
}
- g_signal_connect (conn, "disconnected",
- G_CALLBACK (connection_closed), sa);
-
- /* @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.
+ /* Even if the connection failed, we run the queue, since
+ * there may have been messages waiting for the connection
+ * count to go down.
*/
- soup_connection_release (conn);
- run_queue (sa);
+ do_idle_run_queue (sa);
+ g_object_unref (sa);
}
static gboolean
if (is_new) {
soup_connection_connect_async (conn, got_connection,
- session);
+ g_object_ref (session));
} else
soup_connection_send_request (conn, msg);
}
SoupSessionAsyncQueueData *saqd = user_data;
SoupSessionAsync *sa = saqd->sa;
- g_object_add_weak_pointer (G_OBJECT (sa), (gpointer)&sa);
-
+ g_object_ref (sa);
if (!SOUP_MESSAGE_IS_STARTING (req)) {
g_signal_handlers_disconnect_by_func (req, final_finished, saqd);
if (saqd->callback) {
saqd->callback ((SoupSession *)sa, req,
saqd->callback_data);
- /* callback might destroy sa */
}
g_object_unref (req);
g_slice_free (SoupSessionAsyncQueueData, saqd);
}
- if (sa) {
- g_object_remove_weak_pointer (G_OBJECT (sa), (gpointer)&sa);
- run_queue (sa);
- }
+ do_idle_run_queue (sa);
+ g_object_unref (sa);
}
static gboolean
-idle_run_queue (gpointer user_data)
+idle_run_queue (gpointer sa)
{
- SoupSessionAsync *sa = user_data;
+ SoupSessionAsyncPrivate *priv = SOUP_SESSION_ASYNC_GET_PRIVATE (sa);
- g_object_add_weak_pointer (G_OBJECT (sa), (gpointer)&sa);
- g_object_unref (sa);
-
- if (sa) {
- g_object_remove_weak_pointer (G_OBJECT (sa), (gpointer)&sa);
- run_queue (sa);
- }
+ priv->idle_run_queue_source = NULL;
+ run_queue (sa);
return FALSE;
}
static void
-do_idle_run_queue (SoupSession *session)
+do_idle_run_queue (SoupSessionAsync *sa)
{
- soup_add_completion (soup_session_get_async_context (session),
- idle_run_queue, g_object_ref (session));
+ SoupSessionAsyncPrivate *priv = SOUP_SESSION_ASYNC_GET_PRIVATE (sa);
+
+ if (!priv->idle_run_queue_source) {
+ priv->idle_run_queue_source = soup_add_completion (
+ soup_session_get_async_context ((SoupSession *)sa),
+ idle_run_queue, sa);
+ }
}
static void
G_CALLBACK (final_finished), saqd);
SOUP_SESSION_CLASS (soup_session_async_parent_class)->queue_message (session, req, callback, user_data);
- do_idle_run_queue (session);
+ do_idle_run_queue (sa);
}
static guint
* any messages waiting for this host, since they're out
* of luck.
*/
+ g_object_ref (session);
for (msg = soup_message_queue_first (priv->queue, &iter); msg; msg = soup_message_queue_next (priv->queue, &iter)) {
if (get_host_for_message (session, msg) == host) {
if (status == SOUP_STATUS_TRY_AGAIN) {
}
}
}
+ g_object_unref (session);
}
/**
errors++;
}
- soup_session_abort (session);
- g_object_unref (session);
+ soup_test_session_abort_unref (session);
g_object_unref (msg1);
g_object_unref (msg3);
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);
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);
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");
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);
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
}
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);
}
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);
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 *))) {
/* 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)
{
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",
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)
{
soup_server_get_port (server));
do_host_test ();
+ do_callback_unref_test ();
g_free (base_uri);
user != NULL ? SOUP_STATUS_OK :
SOUP_STATUS_UNAUTHORIZED);
- soup_session_abort (session);
- g_object_unref (session);
+ soup_test_session_abort_unref (session);
}
static void
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
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);
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);
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);
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
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 ();
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
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 *
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);
} 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);
+ }
+}
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);
if (!test_fault_args ())
errors++;
- soup_session_abort (session);
- g_object_unref (session);
+ soup_test_session_abort_unref (session);
test_cleanup ();
return errors != 0;
projects / platform / upstream / libsoup.git / commitdiff