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

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++;
 	}