don't leak the async_context
authorDan Winship <danw@src.gnome.org>
Wed, 21 Nov 2007 04:15:53 +0000 (04:15 +0000)
committerDan Winship <danw@src.gnome.org>
Wed, 21 Nov 2007 04:15:53 +0000 (04:15 +0000)
* libsoup/soup-message-io.c (soup_message_io_unpause): don't leak
the async_context

* libsoup/soup-server.c (soup_server_quit): disconnect the
"new_connection" handler.
(soup_server_get_async_context): Convenience method to return the
server's async_context.

* libsoup/soup-server-message.c: don't circularly ref the server,
there's no need anyway.

* libsoup/soup-session.c (soup_session_get_async_context):
Convenience method to return the session's async_context.

* libsoup/soup-session-async.c (queue_message): call run_queue in
the session's async_context, not the main context.
(send_message): don't leak the async_context

* libsoup/soup-session-sync.c (queue_message_thread): don't leak
the async_context

* tests/context-test.c: test that SOUP_SESSION_ASYNC_CONTEXT works
and doesn't leak

svn path=/trunk/; revision=954

ChangeLog
libsoup/soup-message-io.c
libsoup/soup-server-message.c
libsoup/soup-server.c
libsoup/soup-server.h
libsoup/soup-session-async.c
libsoup/soup-session-sync.c
libsoup/soup-session.c
libsoup/soup-session.h
tests/Makefile.am
tests/context-test.c [new file with mode: 0644]

index d2874c9..86968f1 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,31 @@
 2007-11-20  Dan Winship  <danw@gnome.org>
 
+       * libsoup/soup-message-io.c (soup_message_io_unpause): don't leak
+       the async_context
+
+       * libsoup/soup-server.c (soup_server_quit): disconnect the
+       "new_connection" handler.
+       (soup_server_get_async_context): Convenience method to return the
+       server's async_context.
+
+       * libsoup/soup-server-message.c: don't circularly ref the server,
+       there's no need anyway.
+
+       * libsoup/soup-session.c (soup_session_get_async_context):
+       Convenience method to return the session's async_context.
+
+       * libsoup/soup-session-async.c (queue_message): call run_queue in
+       the session's async_context, not the main context.
+       (send_message): don't leak the async_context
+
+       * libsoup/soup-session-sync.c (queue_message_thread): don't leak
+       the async_context
+
+       * tests/context-test.c: test that SOUP_SESSION_ASYNC_CONTEXT works
+       and doesn't leak
+
+2007-11-20  Dan Winship  <danw@gnome.org>
+
        * libsoup/soup-connection.c (soup_connection_connect_async): don't
        leak the SoupAddress.
 
index fe4b43b..b9e0991 100644 (file)
@@ -926,6 +926,8 @@ soup_message_io_unpause (SoupMessage *msg)
                soup_add_idle (async_context, io_unpause_internal, msg);
        else
                io_unpause_internal (msg);
+       if (async_context)
+               g_main_context_unref (async_context);
 }
 
 /**
index 47cd4b8..ca4cc1e 100644 (file)
@@ -47,17 +47,6 @@ finalize (GObject *object)
 }
 
 static void
-dispose (GObject *object)
-{
-       SoupServerMessagePrivate *priv = SOUP_SERVER_MESSAGE_GET_PRIVATE (object);
-
-       if (priv->server)
-               g_object_unref (priv->server);
-
-       G_OBJECT_CLASS (soup_server_message_parent_class)->dispose (object);
-}
-
-static void
 soup_server_message_class_init (SoupServerMessageClass *soup_server_message_class)
 {
        GObjectClass *object_class = G_OBJECT_CLASS (soup_server_message_class);
@@ -66,7 +55,6 @@ soup_server_message_class_init (SoupServerMessageClass *soup_server_message_clas
 
        /* virtual method override */
        object_class->finalize = finalize;
-       object_class->dispose = dispose;
 }
 
 
@@ -78,7 +66,7 @@ soup_server_message_new (SoupServer *server)
        g_return_val_if_fail (SOUP_IS_SERVER (server), NULL);
 
        smsg = g_object_new (SOUP_TYPE_SERVER_MESSAGE, NULL);
-       SOUP_SERVER_MESSAGE_GET_PRIVATE (smsg)->server = g_object_ref (server);
+       SOUP_SERVER_MESSAGE_GET_PRIVATE (smsg)->server = server;
 
        return smsg;
 }
index f456b21..051effe 100644 (file)
@@ -523,12 +523,36 @@ soup_server_quit (SoupServer *server)
        g_return_if_fail (SOUP_IS_SERVER (server));
        priv = SOUP_SERVER_GET_PRIVATE (server);
 
+       g_signal_handlers_disconnect_by_func (priv->listen_sock,
+                                             G_CALLBACK (new_connection),
+                                             server);
        if (priv->loop)
                g_main_loop_quit (priv->loop);
 
        g_object_unref (server);
 }
 
+/**
+ * soup_server_get_async_context:
+ * @server: a #SoupServer
+ *
+ * Gets @server's async_context. This does not add a ref to the
+ * context, so you will need to ref it yourself if you want it to
+ * outlive its server.
+ *
+ * Return value: @server's #GMainContext, which may be %NULL
+ **/
+GMainContext *
+soup_server_get_async_context (SoupServer *server)
+{
+       SoupServerPrivate *priv;
+
+       g_return_val_if_fail (SOUP_IS_SERVER (server), NULL);
+       priv = SOUP_SERVER_GET_PRIVATE (server);
+
+       return priv->async_context;
+}
+
 static void
 append_handler (gpointer key, gpointer value, gpointer user_data)
 {
index 83cb8cc..aab2769 100644 (file)
@@ -80,6 +80,7 @@ void               soup_server_run            (SoupServer            *serv);
 void               soup_server_run_async      (SoupServer            *serv);
 void               soup_server_quit           (SoupServer            *serv);
 
+GMainContext      *soup_server_get_async_context (SoupServer         *serv);
 
 /* Handlers */
 
index 614928c..a5634f2 100644 (file)
@@ -11,6 +11,7 @@
 
 #include "soup-session-async.h"
 #include "soup-connection.h"
+#include "soup-misc.h"
 
 static gboolean run_queue (SoupSessionAsync *sa, gboolean try_pruning);
 
@@ -218,17 +219,15 @@ queue_message (SoupSession *session, SoupMessage *req,
        SOUP_SESSION_CLASS (soup_session_async_parent_class)->queue_message (session, req, callback, user_data);
 
        g_object_ref (sa);
-       g_idle_add (idle_run_queue, sa);
+       soup_add_idle (soup_session_get_async_context (session),
+                      idle_run_queue, sa);
 }
 
 static guint
 send_message (SoupSession *session, SoupMessage *req)
 {
-       GMainContext *async_context;
-
-       g_object_get (G_OBJECT (session),
-                     SOUP_SESSION_ASYNC_CONTEXT, &async_context,
-                     NULL);
+       GMainContext *async_context =
+               soup_session_get_async_context (session);
 
        /* Balance out the unref that final_finished will do */
        g_object_ref (req);
index b6abbdf..b45408e 100644 (file)
@@ -131,12 +131,8 @@ queue_message_thread (gpointer data)
 
        soup_session_send_message (sad->session, sad->msg);
        if (sad->callback) {
-               GMainContext *async_context;
-
-               g_object_get (sad->session,
-                             SOUP_SESSION_ASYNC_CONTEXT, &async_context,
-                             NULL);
-               soup_add_idle (async_context, queue_message_callback, sad);
+               soup_add_idle (soup_session_get_async_context (sad->session),
+                              queue_message_callback, sad);
        } else
                async_data_free (sad);
 
index ae8da80..37c1900 100644 (file)
@@ -531,6 +531,26 @@ soup_session_remove_filter (SoupSession *session, SoupMessageFilter *filter)
        g_object_unref (filter);
 }
 
+/**
+ * soup_session_get_async_context:
+ * @session: a #SoupSession
+ *
+ * Gets @session's async_context. This does not add a ref to the
+ * context, so you will need to ref it yourself if you want it to
+ * outlive its session.
+ *
+ * Return value: @session's #GMainContext, which may be %NULL
+ **/
+GMainContext *
+soup_session_get_async_context (SoupSession *session)
+{
+       SoupSessionPrivate *priv;
+
+       g_return_val_if_fail (SOUP_IS_SESSION (session), NULL);
+       priv = SOUP_SESSION_GET_PRIVATE (session);
+
+       return priv->async_context;
+}
 
 /* Hosts */
 static guint
index 03b356a..38ce382 100644 (file)
@@ -63,6 +63,8 @@ void            soup_session_add_filter       (SoupSession           *session,
 void            soup_session_remove_filter    (SoupSession           *session,
                                               SoupMessageFilter     *filter);
 
+GMainContext   *soup_session_get_async_context(SoupSession           *session);
+
 void            soup_session_queue_message    (SoupSession           *session,
                                               SoupMessage           *msg,
                                               SoupMessageCallbackFn  callback,
index 9bc5537..9929507 100644 (file)
@@ -8,6 +8,7 @@ INCLUDES =              \
 LIBS = $(top_builddir)/libsoup/libsoup-$(SOUP_API_VERSION).la
 
 noinst_PROGRAMS =      \
+       context-test    \
        date            \
        dict            \
        dns             \
@@ -24,6 +25,7 @@ noinst_PROGRAMS =     \
        $(XMLRPC_TESTS)
 
 auth_test_SOURCES = auth-test.c apache-wrapper.c apache-wrapper.h
+context_test_SOURCES = context-test.c
 date_SOURCES = date.c
 dict_SOURCES = dict.c
 dns_SOURCES = dns.c
@@ -50,7 +52,7 @@ if HAVE_XMLRPC_EPI_PHP
 XMLRPC_TESTS = xmlrpc-test
 endif
 
-TESTS = date header-parsing uri-parsing ntlm-test $(APACHE_TESTS) $(SSL_TESTS) $(XMLRPC_TESTS)
+TESTS = context-test date header-parsing uri-parsing ntlm-test $(APACHE_TESTS) $(SSL_TESTS) $(XMLRPC_TESTS)
 
 EXTRA_DIST =           \
        libsoup.supp    \
diff --git a/tests/context-test.c b/tests/context-test.c
new file mode 100644 (file)
index 0000000..b44f070
--- /dev/null
@@ -0,0 +1,366 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */
+/*
+ * Copyright (C) 2007 Red Hat, Inc.
+ */
+
+#include <ctype.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <glib.h>
+#include <libsoup/soup-address.h>
+#include <libsoup/soup-message.h>
+#include <libsoup/soup-misc.h>
+#include <libsoup/soup-server.h>
+#include <libsoup/soup-server-message.h>
+#include <libsoup/soup-session-async.h>
+#include <libsoup/soup-session-sync.h>
+
+gboolean debug = FALSE;
+int errors = 0;
+GThread *server_thread;
+char *base_uri;
+
+static void
+dprintf (const char *format, ...)
+{
+       va_list args;
+
+       if (!debug)
+               return;
+
+       va_start (args, format);
+       vprintf (format, args);
+       va_end (args);
+}
+
+static void
+request_failed (SoupMessage *msg, gpointer timeout)
+{
+       if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code))
+               g_source_destroy (timeout);
+}
+
+static gboolean
+add_body_chunk (gpointer data)
+{
+       SoupMessage *msg = data;
+
+       soup_message_add_chunk (msg, SOUP_BUFFER_STATIC,
+                               "OK\r\n", 4);
+       soup_message_add_final_chunk (msg);
+       soup_message_io_unpause (msg);
+       g_object_unref (msg);
+
+       return FALSE;
+}
+
+static void
+server_callback (SoupServerContext *context, SoupMessage *msg, gpointer data)
+{
+       GSource *timeout;
+
+       if (soup_method_get_id (msg->method) != SOUP_METHOD_ID_GET) {
+               soup_message_set_status (msg, SOUP_STATUS_NOT_IMPLEMENTED);
+               return;
+       }
+
+       if (!strcmp (context->path, "/shutdown")) {
+               soup_server_quit (context->server);
+               return;
+       }
+
+       soup_message_set_status (msg, SOUP_STATUS_OK);
+       if (!strcmp (context->path, "/fast")) {
+               soup_message_set_response (msg, "text/plain",
+                                          SOUP_BUFFER_STATIC, "OK\r\n", 4);
+               return;
+       }
+
+       soup_server_message_set_encoding (SOUP_SERVER_MESSAGE (msg),
+                                         SOUP_TRANSFER_CHUNKED);
+       g_object_ref (msg);
+       soup_message_io_pause (msg);
+
+       timeout = soup_add_timeout (
+               soup_server_get_async_context (context->server),
+               200, add_body_chunk, msg);
+       g_signal_connect (msg, "finished",
+                         G_CALLBACK (request_failed), timeout);
+}
+
+static gpointer
+run_server_thread (gpointer user_data)
+{
+       SoupServer *server = user_data;
+
+       soup_server_add_handler (server, NULL, NULL,
+                                server_callback, NULL, NULL);
+       soup_server_run (server);
+       g_object_unref (server);
+
+       return NULL;
+}
+
+static guint
+create_server (void)
+{
+       SoupServer *server;
+       GMainContext *async_context;
+       guint port;
+
+       async_context = g_main_context_new ();
+       server = soup_server_new (SOUP_SERVER_PORT, 0,
+                                 SOUP_SERVER_ASYNC_CONTEXT, async_context,
+                                 NULL);
+       g_main_context_unref (async_context);
+
+       if (!server) {
+               fprintf (stderr, "Unable to bind server\n");
+               exit (1);
+       }
+
+       port = soup_server_get_port (server);
+       server_thread = g_thread_create (run_server_thread, server, TRUE, NULL);
+
+       return port;
+}
+
+static void
+shutdown_server (void)
+{
+       SoupSession *session;
+       char *uri;
+       SoupMessage *msg;
+
+       session = soup_session_sync_new ();
+       uri = g_build_filename (base_uri, "shutdown", NULL);
+       msg = soup_message_new ("GET", uri);
+       soup_session_send_message (session, msg);
+       g_object_unref (msg);
+       g_free (uri);
+
+       soup_session_abort (session);
+       g_object_unref (session);
+
+       g_thread_join (server_thread);
+}
+
+/* Test 1: An async session in another thread with its own
+ * async_context can complete a request while the main thread's main
+ * loop is stopped.
+ */
+
+static gboolean idle_start_test1_thread (gpointer loop);
+static gpointer test1_thread (gpointer user_data);
+
+GCond *test1_cond;
+GMutex *test1_mutex;
+
+static void
+do_test1 (void)
+{
+       GMainLoop *loop;
+
+       dprintf ("Test 1: blocking the main thread does not block other thread\n");
+
+       test1_cond = g_cond_new ();
+       test1_mutex = g_mutex_new ();
+
+       loop = g_main_loop_new (NULL, FALSE);
+       g_idle_add (idle_start_test1_thread, loop);
+       g_main_loop_run (loop);
+       g_main_loop_unref (loop);
+
+       g_mutex_free (test1_mutex);
+       g_cond_free (test1_cond);
+}
+
+static gboolean
+idle_start_test1_thread (gpointer loop)
+{
+       GTimeVal time;
+       GThread *thread;
+
+       g_mutex_lock (test1_mutex);
+       thread = g_thread_create (test1_thread, base_uri, TRUE, NULL);
+
+       g_get_current_time (&time);
+       time.tv_sec += 5;
+       if (g_cond_timed_wait (test1_cond, test1_mutex, &time))
+               g_thread_join (thread);
+       else {
+               dprintf ("  timeout!\n");
+               errors++;
+       }
+
+       g_mutex_unlock (test1_mutex);
+       g_main_loop_quit (loop);
+       return FALSE;
+}
+
+static void
+test1_finished (SoupMessage *msg, gpointer loop)
+{
+       g_main_loop_quit (loop);
+}
+
+static gpointer
+test1_thread (gpointer user_data)
+{
+       SoupSession *session;
+       GMainContext *async_context;
+       char *uri;
+       SoupMessage *msg;
+       GMainLoop *loop;
+
+       /* Wait for main thread to be waiting on test1_cond */
+       g_mutex_lock (test1_mutex);
+       g_mutex_unlock (test1_mutex);
+
+       async_context = g_main_context_new ();
+       session = soup_session_async_new_with_options (
+               SOUP_SESSION_ASYNC_CONTEXT, async_context,
+               NULL);
+       g_main_context_unref (async_context);
+
+       uri = g_build_filename (base_uri, "slow", NULL);
+
+       dprintf ("  send_message\n");
+       msg = soup_message_new ("GET", uri);
+       soup_session_send_message (session, msg);
+       if (msg->status_code != SOUP_STATUS_OK) {
+               dprintf ("    unexpected status: %d %s\n",
+                        msg->status_code, msg->reason_phrase);
+               errors++;
+       }
+       g_object_unref (msg);
+
+       dprintf ("  queue_message\n");
+       msg = soup_message_new ("GET", uri);
+       loop = g_main_loop_new (async_context, FALSE);
+       g_object_ref (msg);
+       soup_session_queue_message (session, msg, test1_finished, loop);
+       g_main_loop_run (loop);
+       g_main_loop_unref (loop);
+       if (msg->status_code != SOUP_STATUS_OK) {
+               dprintf ("    unexpected status: %d %s\n",
+                        msg->status_code, msg->reason_phrase);
+               errors++;
+       }
+       g_object_unref (msg);
+
+       soup_session_abort (session);
+       g_object_unref (session);
+       g_free (uri);
+
+       g_cond_signal (test1_cond);
+       return NULL;
+}
+
+/* Test 2: An async session in the main thread with its own
+ * async_context runs independently of the default main loop.
+ */
+
+static gboolean idle_test2_fail (gpointer user_data);
+
+static void
+do_test2 (void)
+{
+       guint idle;
+       GMainContext *async_context;
+       SoupSession *session;
+       char *uri;
+       SoupMessage *msg;
+
+       dprintf ("Test 2: a session with its own context is independent of the main loop.\n");
+
+       idle = g_idle_add_full (G_PRIORITY_HIGH, idle_test2_fail, NULL, NULL);
+
+       async_context = g_main_context_new ();
+       session = soup_session_async_new_with_options (
+               SOUP_SESSION_ASYNC_CONTEXT, async_context,
+               NULL);
+       g_main_context_unref (async_context);
+
+       uri = g_build_filename (base_uri, "slow", NULL);
+
+       dprintf ("  send_message\n");
+       msg = soup_message_new ("GET", uri);
+       soup_session_send_message (session, msg);
+       if (msg->status_code != SOUP_STATUS_OK) {
+               dprintf ("    unexpected status: %d %s\n",
+                        msg->status_code, msg->reason_phrase);
+               errors++;
+       }
+       g_object_unref (msg);
+
+       soup_session_abort (session);
+       g_object_unref (session);
+       g_free (uri);
+
+       g_source_remove (idle);
+}
+
+static gboolean
+idle_test2_fail (gpointer user_data)
+{
+       dprintf ("  idle ran!\n");
+       errors++;
+       return FALSE;
+}
+
+
+static void
+quit (int sig)
+{
+       /* Exit cleanly on ^C in case we're valgrinding. */
+       exit (0);
+}
+
+int
+main (int argc, char **argv)
+{
+       int opt;
+       guint port;
+
+       g_type_init ();
+       g_thread_init (NULL);
+       signal (SIGINT, quit);
+
+       while ((opt = getopt (argc, argv, "d")) != -1) {
+               switch (opt) {
+               case 'd':
+                       debug = TRUE;
+                       break;
+               default:
+                       fprintf (stderr, "Usage: %s [-d]\n",
+                                argv[0]);
+                       exit (1);
+               }
+       }
+
+       port = create_server ();
+       base_uri = g_strdup_printf ("http://localhost:%u/", port);
+
+       do_test1 ();
+       do_test2 ();
+
+       shutdown_server ();
+       g_free (base_uri);
+       g_main_context_unref (g_main_context_default ());
+
+       dprintf ("\n");
+       if (errors) {
+               printf ("context-test: %d error(s). Run with '-d' for details\n",
+                       errors);
+       } else
+               printf ("context-test: OK\n");
+       return errors != 0;
+}