diff options
author | Dan Winship <danw@gnome.org> | 2013-09-29 09:06:37 -0400 |
---|---|---|
committer | Dan Winship <danw@gnome.org> | 2013-09-29 10:45:13 -0400 |
commit | 96da2df64c9dd8cc52e97ce73e54615d6b520664 (patch) | |
tree | 9a3047c4779b22af29f179a6195daa5db4ba2003 | |
parent | a5d3351caf88824db7de968f312565851d47840a (diff) | |
download | libsoup-96da2df64c9dd8cc52e97ce73e54615d6b520664.tar.gz |
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
-rw-r--r-- | libsoup/soup-client-input-stream.c | 12 | ||||
-rw-r--r-- | libsoup/soup-message-io.c | 10 | ||||
-rw-r--r-- | tests/redirect-test.c | 8 | ||||
-rw-r--r-- | tests/requester-test.c | 94 | ||||
-rw-r--r-- | tests/test-utils.c | 33 | ||||
-rw-r--r-- | tests/test-utils.h | 4 | ||||
-rw-r--r-- | 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 d73fb007..87fa49d6 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 f5f4c514..16a74e11 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 9bc4621d..2b4fb5ea 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 6e169ad0..cfa9fcd6 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 0b739393..9848d9ba 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 98d93393..c85103d8 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 405ec3c1..5903069b 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) { |