diff options
author | Dan Winship <danw@gnome.org> | 2013-11-04 16:41:10 -0500 |
---|---|---|
committer | Dan Winship <danw@gnome.org> | 2013-11-04 16:48:41 -0500 |
commit | 4e755ba87e2937ce32c53c2983d4efe8b9121541 (patch) | |
tree | 7dfb8b0289835219256fe084198315024cc83905 | |
parent | da167aad39438d4df33df189c97bd07e989d38d0 (diff) | |
download | libsoup-4e755ba87e2937ce32c53c2983d4efe8b9121541.tar.gz |
Fix SoupClientInputStream close on error
g_input/output_stream_close() always mark their stream closed, even on
error, even if their cancellable is pre-cancelled. That means we have
to guarantee that soup_message_io_finished() gets called when a
SoupClientInputStream is closed, even if an error occurs while
closing.
https://bugzilla.gnome.org/show_bug.cgi?id=711260
-rw-r--r-- | libsoup/soup-client-input-stream.c | 9 | ||||
-rw-r--r-- | libsoup/soup-message-io.c | 25 | ||||
-rw-r--r-- | tests/requester-test.c | 54 |
3 files changed, 72 insertions, 16 deletions
diff --git a/libsoup/soup-client-input-stream.c b/libsoup/soup-client-input-stream.c index 87fa49d6..0264cb79 100644 --- a/libsoup/soup-client-input-stream.c +++ b/libsoup/soup-client-input-stream.c @@ -128,9 +128,12 @@ soup_client_input_stream_close_fn (GInputStream *stream, GError **error) { SoupClientInputStream *cistream = SOUP_CLIENT_INPUT_STREAM (stream); + gboolean success; - return soup_message_io_run_until_finish (cistream->priv->msg, TRUE, - cancellable, error); + success = soup_message_io_run_until_finish (cistream->priv->msg, TRUE, + NULL, error); + soup_message_io_finished (cistream->priv->msg); + return success; } static gboolean @@ -158,6 +161,8 @@ close_async_ready (SoupMessage *msg, gpointer user_data) return TRUE; } + soup_message_io_finished (cistream->priv->msg); + if (error) { g_task_return_error (task, error); g_object_unref (task); diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c index 6ba29607..be5cb2d2 100644 --- a/libsoup/soup-message-io.c +++ b/libsoup/soup-message-io.c @@ -155,8 +155,14 @@ soup_message_io_finished (SoupMessage *msg) { SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg); SoupMessageIOData *io = priv->io_data; - SoupMessageCompletionFn completion_cb = io->completion_cb; - gpointer completion_data = io->completion_data; + SoupMessageCompletionFn completion_cb; + gpointer completion_data; + + if (!io) + return; + + completion_cb = io->completion_cb; + completion_data = io->completion_data; g_object_ref (msg); soup_message_io_cleanup (msg); @@ -984,6 +990,7 @@ soup_message_io_run_until_finish (SoupMessage *msg, { SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg); SoupMessageIOData *io = priv->io_data; + gboolean success; g_object_ref (msg); @@ -994,17 +1001,13 @@ soup_message_io_run_until_finish (SoupMessage *msg, io->read_state = SOUP_MESSAGE_IO_STATE_FINISHING; } - if (!io_run_until (msg, blocking, - SOUP_MESSAGE_IO_STATE_DONE, - SOUP_MESSAGE_IO_STATE_DONE, - cancellable, error)) { - g_object_unref (msg); - return FALSE; - } + success = io_run_until (msg, blocking, + SOUP_MESSAGE_IO_STATE_DONE, + SOUP_MESSAGE_IO_STATE_DONE, + cancellable, error); - soup_message_io_finished (msg); g_object_unref (msg); - return TRUE; + return success; } static void diff --git a/tests/requester-test.c b/tests/requester-test.c index 461f1934..a202c164 100644 --- a/tests/requester-test.c +++ b/tests/requester-test.c @@ -788,6 +788,15 @@ do_null_char_tests (void) } static void +close_test_msg_finished (SoupMessage *msg, + gpointer user_data) +{ + gboolean *finished = user_data; + + *finished = TRUE; +} + +static void do_close_test_for_session (SoupSession *session, SoupURI *uri) { @@ -795,12 +804,17 @@ do_close_test_for_session (SoupSession *session, GInputStream *stream; SoupRequest *request; guint64 start, end; + GCancellable *cancellable; + SoupMessage *msg; + gboolean finished = FALSE; + + debug_printf (1, " normal close\n"); 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); + debug_printf (1, " could not send request: %s\n", error->message); errors++; g_error_free (error); g_object_unref (request); @@ -810,14 +824,48 @@ do_close_test_for_session (SoupSession *session, 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); + 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"); + debug_printf (1, " close() waited for response to complete!\n"); + errors++; + } + + g_object_unref (stream); + g_object_unref (request); + + + debug_printf (1, " error close\n"); + + request = soup_session_request_uri (session, uri, NULL); + msg = soup_request_http_get_message (SOUP_REQUEST_HTTP (request)); + g_signal_connect (msg, "finished", G_CALLBACK (close_test_msg_finished), &finished); + g_object_unref (msg); + + 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; + } + + cancellable = g_cancellable_new (); + g_cancellable_cancel (cancellable); + soup_test_request_close_stream (request, stream, cancellable, &error); + if (error && !g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { + debug_printf (1, " did not get expected error: %s\n", error->message); + errors++; + g_clear_error (&error); + } + + if (!finished) { + debug_printf (1, " message did not finish!\n"); errors++; } |