[Spice-commits] 5 commits - src/channel-webdav.c src/giopipe.c tests/pipe.c

Marc-André Lureau elmarco at kemper.freedesktop.org
Mon Sep 4 10:20:56 UTC 2017


 src/channel-webdav.c |   17 +++++----
 src/giopipe.c        |    8 +++-
 tests/pipe.c         |   95 +++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 105 insertions(+), 15 deletions(-)

New commits:
commit a073ecacde37082f7f948402dda0dac0ff916d01
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Aug 10 18:10:39 2017 +0200

    webdav: remove the client if input stream is closed
    
    Instead of printing a warning when trying to read from a closed
    stream. It can happen that libsoup closes the pipe output stream while
    the input stream had no pending operation, in which case the close is
    successful and we shouldn't try to read from the stream anymore.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/src/channel-webdav.c b/src/channel-webdav.c
index 4a246b5..f1b6c2a 100644
--- a/src/channel-webdav.c
+++ b/src/channel-webdav.c
@@ -218,7 +218,7 @@ client_ref(Client *client)
     return client;
 }
 
-static void client_start_read(Client *client);
+static bool client_start_read(Client *client);
 
 static void remove_client(Client *client)
 {
@@ -234,10 +234,9 @@ static void mux_pushed_cb(OutputQueue *q, gpointer user_data)
 {
     Client *client = user_data;
 
-    if (client->mux.size == 0) {
+    if (client->mux.size == 0 ||
+        !client_start_read(client)) {
         remove_client(client);
-    } else {
-        client_start_read(client);
     }
 
     client_unref(client);
@@ -280,14 +279,18 @@ end:
     client_unref(client);
 }
 
-static void client_start_read(Client *client)
+static bool client_start_read(Client *client)
 {
     GInputStream *input;
 
     input = g_io_stream_get_input_stream(G_IO_STREAM(client->pipe));
+    if (g_input_stream_is_closed(input)) {
+        return false;
+    }
     g_input_stream_read_async(input, client->mux.buf, MAX_MUX_SIZE,
                               G_PRIORITY_DEFAULT, client->cancellable, server_reply_cb,
                               client_ref(client));
+    return true;
 }
 
 static void start_demux(SpiceWebdavChannel *self);
@@ -361,6 +364,7 @@ static void start_client(SpiceWebdavChannel *self)
     SoupServer *server;
     GSocketAddress *addr;
     GError *error = NULL;
+    bool started;
 
     session = spice_channel_get_session(SPICE_CHANNEL(self));
     server = phodav_server_get_soup_server(spice_session_get_webdav_server(session));
@@ -384,7 +388,8 @@ G_GNUC_END_IGNORE_DEPRECATIONS
 
     g_hash_table_insert(c->clients, &client->id, client);
 
-    client_start_read(client);
+    started = client_start_read(client);
+    g_assert(started);
     demux_to_client(client);
 
     g_clear_object(&addr);
commit 35701d818f6e26b102a35f06ba68b2f334ba10fb
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Aug 17 15:20:10 2017 +0200

    tests/pipe: add write close/cancel tests
    
    And a few cleanups.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/tests/pipe.c b/tests/pipe.c
index 7961f4f..1fe5ba3 100644
--- a/tests/pipe.c
+++ b/tests/pipe.c
@@ -210,6 +210,7 @@ readclose_cb(GObject *source, GAsyncResult *result, gpointer user_data)
     nbytes = g_input_stream_read_finish(G_INPUT_STREAM(source), result, &error);
 
     g_assert_cmpint(nbytes, ==, 0);
+    g_assert_no_error(error);
 
     g_main_loop_quit (loop);
 }
@@ -222,6 +223,7 @@ test_pipe_readclosestream(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
     g_input_stream_read_async(f->ip2, f->buf, 1, G_PRIORITY_DEFAULT,
                               f->cancellable, readclose_cb, f->loop);
     g_io_stream_close(f->p1, f->cancellable, &error);
+    g_assert_no_error(error);
 
     g_main_loop_run (f->loop);
 }
@@ -234,6 +236,7 @@ test_pipe_readclose(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
     g_input_stream_read_async(f->ip2, f->buf, 1, G_PRIORITY_DEFAULT,
                               f->cancellable, readclose_cb, f->loop);
     g_output_stream_close(f->op1, f->cancellable, &error);
+    g_assert_no_error(error);
 
     g_main_loop_run (f->loop);
 }
@@ -257,8 +260,6 @@ readcancel_cb(GObject *source, GAsyncResult *result, gpointer user_data)
 static void
 test_pipe_readcancel(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
 {
-    GError *error = NULL;
-
     g_input_stream_read_async(f->ip2, f->buf, 1, G_PRIORITY_DEFAULT,
                               f->cancellable, readcancel_cb, f->loop);
     g_cancellable_cancel(f->cancellable);
@@ -266,6 +267,76 @@ test_pipe_readcancel(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
     g_main_loop_run (f->loop);
 }
 
+static void
+writeclose_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    GError *error = NULL;
+    gssize nbytes;
+    GMainLoop *loop = user_data;
+
+    nbytes = g_output_stream_write_finish(G_OUTPUT_STREAM(source),
+                                          result, &error);
+
+    g_assert_cmpint(nbytes, ==, -1);
+    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CLOSED);
+    g_clear_error(&error);
+
+    g_main_loop_quit (loop);
+}
+
+static void
+test_pipe_writeclosestream(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
+{
+    GError *error = NULL;
+
+    g_output_stream_write_async(f->op1, f->buf, 1, G_PRIORITY_DEFAULT,
+                                f->cancellable, writeclose_cb, f->loop);
+    g_io_stream_close(f->p2, f->cancellable, &error);
+    g_assert_no_error(error);
+
+    g_main_loop_run (f->loop);
+}
+
+static void
+test_pipe_writeclose(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
+{
+    GError *error = NULL;
+
+    g_output_stream_write_async(f->op1, f->buf, 1, G_PRIORITY_DEFAULT,
+                                f->cancellable, writeclose_cb, f->loop);
+    g_input_stream_close(f->ip2, f->cancellable, &error);
+    g_assert_no_error(error);
+
+    g_main_loop_run (f->loop);
+}
+
+static void
+writecancel_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    GError *error = NULL;
+    gssize nbytes;
+    GMainLoop *loop = user_data;
+
+    nbytes = g_output_stream_write_finish(G_OUTPUT_STREAM(source),
+                                          result, &error);
+
+    g_assert_cmpint(nbytes, ==, -1);
+    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CANCELLED);
+    g_clear_error(&error);
+
+    g_main_loop_quit (loop);
+}
+
+static void
+test_pipe_writecancel(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
+{
+    g_output_stream_write_async(f->op1, f->buf, 1, G_PRIORITY_DEFAULT,
+                                f->cancellable, writecancel_cb, f->loop);
+    g_cancellable_cancel(f->cancellable);
+
+    g_main_loop_run (f->loop);
+}
+
 static gchar *
 get_test_data(gint n)
 {
@@ -522,5 +593,17 @@ int main(int argc, char* argv[])
                fixture_set_up, test_pipe_readcancel,
                fixture_tear_down);
 
+    g_test_add("/pipe/writeclosestream", Fixture, NULL,
+               fixture_set_up, test_pipe_writeclosestream,
+               fixture_tear_down);
+
+    g_test_add("/pipe/writeclose", Fixture, NULL,
+               fixture_set_up, test_pipe_writeclose,
+               fixture_tear_down);
+
+    g_test_add("/pipe/writecancel", Fixture, NULL,
+               fixture_set_up, test_pipe_writecancel,
+               fixture_tear_down);
+
     return g_test_run();
 }
commit cef1eb8b9a5608069f62a4e319e3b4a655d6fe80
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Aug 17 15:18:32 2017 +0200

    giopipe: output wake up on peer close
    
    Similar to input is_readable(), return writable after a close, so an
    error is reported to the async write.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/src/giopipe.c b/src/giopipe.c
index efe9857..c653cb8 100644
--- a/src/giopipe.c
+++ b/src/giopipe.c
@@ -420,7 +420,7 @@ pipe_output_stream_is_writable (GPollableOutputStream *stream)
     PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
     gboolean writable;
 
-    writable = self->buffer == NULL || self->peer->read >= 0;
+    writable = self->buffer == NULL || self->peer->read >= 0 || self->peer_closed;
     //g_debug("writable %p %d", self, writable);
 
     return writable;
commit 1dd19d52a33e1946bf135b56f8d69d15800bfc14
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Aug 10 18:11:10 2017 +0200

    giopipe: input return 0 and no error when peer is already closed
    
    The peer stream may be closed, but may have failed to close self,
    because it has pending operations. In this case indicate EOS on read
    rather than returning an error. self will be close eventually on
    dispose.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/src/giopipe.c b/src/giopipe.c
index 08aea9a..efe9857 100644
--- a/src/giopipe.c
+++ b/src/giopipe.c
@@ -98,7 +98,11 @@ pipe_input_stream_read (GInputStream  *stream,
 
     g_return_val_if_fail(count > 0, -1);
 
-    if (g_input_stream_is_closed (stream) || self->peer_closed) {
+    if (self->peer_closed) {
+        return 0;
+    }
+
+    if (g_input_stream_is_closed (stream)) {
         g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
                              "Stream is already closed");
         return -1;
diff --git a/tests/pipe.c b/tests/pipe.c
index a25e79b..7961f4f 100644
--- a/tests/pipe.c
+++ b/tests/pipe.c
@@ -209,9 +209,7 @@ readclose_cb(GObject *source, GAsyncResult *result, gpointer user_data)
 
     nbytes = g_input_stream_read_finish(G_INPUT_STREAM(source), result, &error);
 
-    g_assert_cmpint(nbytes, ==, -1);
-    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CLOSED);
-    g_clear_error(&error);
+    g_assert_cmpint(nbytes, ==, 0);
 
     g_main_loop_quit (loop);
 }
commit 94527a7e207a17e4dbc91690202263e940f18e78
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Aug 10 18:55:28 2017 +0200

    test-pipe: /pipe/readcancel is supposed to test cancel
    
    Not to reproduce /pipe/readclose..
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
    Acked-by: Victor Toso <victortoso at redhat.com>

diff --git a/tests/pipe.c b/tests/pipe.c
index 7a0dafd..a25e79b 100644
--- a/tests/pipe.c
+++ b/tests/pipe.c
@@ -250,7 +250,7 @@ readcancel_cb(GObject *source, GAsyncResult *result, gpointer user_data)
     nbytes = g_input_stream_read_finish(G_INPUT_STREAM(source), result, &error);
 
     g_assert_cmpint(nbytes, ==, -1);
-    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CLOSED);
+    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CANCELLED);
     g_clear_error(&error);
 
     g_main_loop_quit (loop);
@@ -263,7 +263,7 @@ test_pipe_readcancel(Fixture *f, gconstpointer user_data G_GNUC_UNUSED)
 
     g_input_stream_read_async(f->ip2, f->buf, 1, G_PRIORITY_DEFAULT,
                               f->cancellable, readcancel_cb, f->loop);
-    g_output_stream_close(f->op1, f->cancellable, &error);
+    g_cancellable_cancel(f->cancellable);
 
     g_main_loop_run (f->loop);
 }


More information about the Spice-commits mailing list