[libsoup] Fix soup_client_input_stream_close to not block
- From: Dan Winship <danw src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [libsoup] Fix soup_client_input_stream_close to not block
- Date: Sun, 29 Sep 2013 14:47:03 +0000 (UTC)
commit 96da2df64c9dd8cc52e97ce73e54615d6b520664
Author: Dan Winship <danw gnome org>
Date: Sun Sep 29 09:06:37 2013 -0400
Fix soup_client_input_stream_close to not block
Closing a SoupClientInputStream for a message that hadn't been
completely read was trying to read to the end of the message first.
Fix it to just cancel the read instead.
Also fix a few tests that were implicitly assuming the old behavior.
https://bugzilla.gnome.org/show_bug.cgi?id=695652
libsoup/soup-client-input-stream.c | 12 +++--
libsoup/soup-message-io.c | 10 ++++
tests/redirect-test.c | 8 +++
tests/requester-test.c | 94 +++++++++++++++++++++++++++++++++++-
tests/test-utils.c | 33 +++++++++++++
tests/test-utils.h | 4 ++
tests/timeout-test.c | 9 ++++
7 files changed, 163 insertions(+), 7 deletions(-)
---
diff --git a/libsoup/soup-client-input-stream.c b/libsoup/soup-client-input-stream.c
index d73fb00..87fa49d 100644
--- a/libsoup/soup-client-input-stream.c
+++ b/libsoup/soup-client-input-stream.c
@@ -187,11 +187,13 @@ soup_client_input_stream_close_async (GInputStream *stream,
task = g_task_new (stream, cancellable, callback, user_data);
g_task_set_priority (task, priority);
- source = soup_message_io_get_source (cistream->priv->msg,
- cancellable, NULL, NULL);
-
- g_task_attach_source (task, source, (GSourceFunc) close_async_ready);
- g_source_unref (source);
+ if (close_async_ready (cistream->priv->msg, task) == G_SOURCE_CONTINUE) {
+ source = soup_message_io_get_source (cistream->priv->msg,
+ cancellable, NULL, NULL);
+
+ g_task_attach_source (task, source, (GSourceFunc) close_async_ready);
+ g_source_unref (source);
+ }
}
static gboolean
diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c
index f5f4c51..16a74e1 100644
--- a/libsoup/soup-message-io.c
+++ b/libsoup/soup-message-io.c
@@ -982,8 +982,18 @@ soup_message_io_run_until_finish (SoupMessage *msg,
GCancellable *cancellable,
GError **error)
{
+ SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg);
+ SoupMessageIOData *io = priv->io_data;
+
g_object_ref (msg);
+ if (io) {
+ g_return_if_fail (io->mode == SOUP_MESSAGE_IO_CLIENT);
+
+ if (io->read_state < SOUP_MESSAGE_IO_STATE_BODY_DONE)
+ io->read_state = SOUP_MESSAGE_IO_STATE_BODY_DONE;
+ }
+
if (!io_run_until (msg, blocking,
SOUP_MESSAGE_IO_STATE_DONE,
SOUP_MESSAGE_IO_STATE_DONE,
diff --git a/tests/redirect-test.c b/tests/redirect-test.c
index 9bc4621..2b4fb5e 100644
--- a/tests/redirect-test.c
+++ b/tests/redirect-test.c
@@ -285,6 +285,14 @@ do_request_api_test (SoupSession *session, SoupURI *base_uri, int n)
return;
}
+ soup_test_request_read_all (SOUP_REQUEST (reqh), stream, NULL, &error);
+ if (error) {
+ debug_printf (1, " could not read from stream: %s\n",
+ error->message);
+ g_error_free (error);
+ errors++;
+ }
+
soup_test_request_close_stream (SOUP_REQUEST (reqh), stream, NULL, &error);
if (error) {
debug_printf (1, " could not close stream: %s\n",
diff --git a/tests/requester-test.c b/tests/requester-test.c
index 6e169ad..cfa9fcd 100644
--- a/tests/requester-test.c
+++ b/tests/requester-test.c
@@ -37,6 +37,23 @@ get_index (void)
strlen (AUTH_HTML_BODY));
}
+static gboolean
+slow_finish_message (gpointer msg)
+{
+ SoupServer *server = g_object_get_data (G_OBJECT (msg), "server");
+
+ soup_server_unpause_message (server, msg);
+ return FALSE;
+}
+
+static void
+slow_pause_message (SoupMessage *msg, gpointer server)
+{
+ soup_server_pause_message (server, msg);
+ soup_add_timeout (soup_server_get_async_context (server),
+ 1000, slow_finish_message, msg);
+}
+
static void
server_callback (SoupServer *server, SoupMessage *msg,
const char *path, GHashTable *query,
@@ -70,6 +87,10 @@ server_callback (SoupServer *server, SoupMessage *msg,
} else if (strcmp (path, "/non-persistent") == 0) {
soup_message_headers_append (msg->response_headers,
"Connection", "close");
+ } else if (!strcmp (path, "/slow")) {
+ g_object_set_data (G_OBJECT (msg), "server", server);
+ g_signal_connect (msg, "wrote-headers",
+ G_CALLBACK (slow_pause_message), server);
}
soup_message_set_status (msg, SOUP_STATUS_OK);
@@ -670,6 +691,7 @@ do_null_char_request (SoupSession *session, const char *encoded_data,
if (error) {
debug_printf (1, " could not send request: %s\n", error->message);
+ errors++;
g_error_free (error);
g_object_unref (request);
soup_uri_free (uri);
@@ -720,8 +742,8 @@ do_null_char_test (gboolean plain_session)
};
static int num_test_cases = G_N_ELEMENTS(test_cases);
- debug_printf (1, "Streaming data URLs containing null chars with %s\n",
- plain_session ? "SoupSession" : "SoupSessionSync");
+ debug_printf (1, "\nStreaming data URLs containing null chars with %s\n",
+ plain_session ? "SoupSession" : "SoupSessionAsync");
session = soup_test_session_new (plain_session ? SOUP_TYPE_SESSION : SOUP_TYPE_SESSION_ASYNC,
SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
@@ -734,6 +756,72 @@ do_null_char_test (gboolean plain_session)
soup_test_session_abort_unref (session);
}
+static void
+do_close_test_for_session (SoupSession *session,
+ SoupURI *uri)
+{
+ GError *error = NULL;
+ GInputStream *stream;
+ SoupRequest *request;
+ guint64 start, end;
+
+ request = soup_session_request_uri (session, uri, NULL);
+ stream = soup_test_request_send (request, NULL, 0, &error);
+
+ if (error) {
+ debug_printf (1, " could not send request: %s\n", error->message);
+ errors++;
+ g_error_free (error);
+ g_object_unref (request);
+ return;
+ }
+
+ start = g_get_monotonic_time ();
+ soup_test_request_close_stream (request, stream, NULL, &error);
+ if (error) {
+ debug_printf (1, " could not close stream: %s\n", error->message);
+ errors++;
+ g_clear_error (&error);
+ }
+ end = g_get_monotonic_time ();
+
+ if (end - start > 500000) {
+ debug_printf (1, " close() waited for response to complete!\n");
+ errors++;
+ }
+
+ g_object_unref (stream);
+ g_object_unref (request);
+}
+
+static void
+do_close_tests (const char *uri)
+{
+ SoupSession *session;
+ SoupURI *slow_uri;
+
+ debug_printf (1, "\nClosing stream before end should cancel\n");
+
+ slow_uri = soup_uri_new (uri);
+ soup_uri_set_path (slow_uri, "/slow");
+
+ debug_printf (1, " async\n");
+ session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC,
+ SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
+ NULL);
+ do_close_test_for_session (session, slow_uri);
+ soup_test_session_abort_unref (session);
+
+ debug_printf (1, " sync\n");
+ session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC,
+ SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
+ NULL);
+ do_close_test_for_session (session, slow_uri);
+ soup_test_session_abort_unref (session);
+
+ soup_uri_free (slow_uri);
+}
+
int
main (int argc, char **argv)
{
@@ -759,6 +847,8 @@ main (int argc, char **argv)
do_sync_test (uri, TRUE);
do_null_char_test (TRUE);
+ do_close_tests (uri);
+
g_free (uri);
soup_buffer_free (response);
soup_buffer_free (auth_response);
diff --git a/tests/test-utils.c b/tests/test-utils.c
index 0b73939..9848d9b 100644
--- a/tests/test-utils.c
+++ b/tests/test-utils.c
@@ -493,6 +493,39 @@ soup_test_request_send (SoupRequest *req,
}
gboolean
+soup_test_request_read_all (SoupRequest *req,
+ GInputStream *stream,
+ GCancellable *cancellable,
+ GError **error)
+{
+ char buf[8192];
+ AsyncAsSyncData data;
+ gsize nread;
+
+ if (!SOUP_IS_SESSION_SYNC (soup_request_get_session (req)))
+ data.loop = g_main_loop_new (g_main_context_get_thread_default (), FALSE);
+
+ do {
+ if (SOUP_IS_SESSION_SYNC (soup_request_get_session (req))) {
+ nread = g_input_stream_read (stream, buf, sizeof (buf),
+ cancellable, error);
+ } else {
+ g_input_stream_read_async (stream, buf, sizeof (buf),
+ G_PRIORITY_DEFAULT, cancellable,
+ async_as_sync_callback, &data);
+ g_main_loop_run (data.loop);
+ nread = g_input_stream_read_finish (stream, data.result, error);
+ g_object_unref (data.result);
+ }
+ } while (nread > 0);
+
+ if (!SOUP_IS_SESSION_SYNC (soup_request_get_session (req)))
+ g_main_loop_unref (data.loop);
+
+ return nread == 0;
+}
+
+gboolean
soup_test_request_close_stream (SoupRequest *req,
GInputStream *stream,
GCancellable *cancellable,
diff --git a/tests/test-utils.h b/tests/test-utils.h
index 98d9339..c85103d 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -44,6 +44,10 @@ GInputStream *soup_test_request_send (SoupRequest *req,
GCancellable *cancellable,
guint flags,
GError **error);
+gboolean soup_test_request_read_all (SoupRequest *req,
+ GInputStream *stream,
+ GCancellable *cancellable,
+ GError **error);
gboolean soup_test_request_close_stream (SoupRequest *req,
GInputStream *stream,
GCancellable *cancellable,
diff --git a/tests/timeout-test.c b/tests/timeout-test.c
index 405ec3c..5903069 100644
--- a/tests/timeout-test.c
+++ b/tests/timeout-test.c
@@ -148,6 +148,15 @@ do_request_to_session (SoupSession *session, const char *uri,
g_clear_error (&error);
if (stream) {
+ soup_test_request_read_all (req, stream, NULL, &error);
+ if (error) {
+ debug_printf (1, " ERROR reading stream: %s\n",
+ error->message);
+ errors++;
+ }
+ }
+
+ if (stream) {
soup_test_request_close_stream (req, stream, NULL, &error);
if (error) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
