Hi ----- Original Message ----- > From: Victor Toso <me@xxxxxxxxxxxxxx> > > The channel can be reset after disabling the shared-folder. If we had > pending read, we should cancel it using the GCancellable for the > demuxing code on vmcstream.c (c->cancellable) and per client operation > (client->cancellable) which is done on client_remove_unref() called by > g_hash_table_remove_all() in port_event() > > This bug resolves rhbz#1474074 for Linux guests, tested on Fedora 25. > > Reproducer: > 1. With remote-viewer, connect to a Fedora 25 (GNOME) with > spice-webavd running; > 2. In remote-viewer, enable shared-folder (File > Preferences); > 3. In the guest, open Nautilus, go to "Other Locations" and double > click at "Spice client folder" to mount that webdav folder; > 4. Wait for the folder to be mounted in the guest; > 5. In remote-viewer, disabled shared-folder; > 6. In the guest, try to access the mounted folder. It should fail and > the mount point will be removed; > 7. Repeat steps 2->3 and see the client crash. > > > Thread 1 "remote-viewer" received signal SIGSEGV, Segmentation fault. > > (gdb) bt full > > #0 in __memmove_avx_unaligned_erms () from /lib64/libc.so.6 > > #1 in spice_channel_read_sasl (channel=0xca8130, data=0xfc2050, len=6) at > > spice-channel.c:1122 > > #2 in spice_channel_read (channel=0xca8130, data=0xfc2050, length=6) at > > spice-channel.c:1149 > > #3 in spice_channel_recv_msg (channel=0xca8130, > msg_handler=0x7ffff4e52e3c <spice_webdav_handle_msg>, data=0x0) at > spice-channel.c:2031 > > #4 in spice_channel_iterate_read (channel=0xca8130) at > > spice-channel.c:2338 > > #5 in spice_channel_iterate (channel=0xca8130) at spice-channel.c:2376 > > #6 in spice_channel_coroutine (data=0xca8130) at spice-channel.c:2664 > > #7 0x00007ffff4e92d49 in coroutine_trampoline (cc=0xca77e0) at > > coroutine_ucontext.c:63 > > #8 0x00007ffff4e92a01 in continuation_trampoline (i0=13268960, i1=0) at > > continuation.c:55 > > #9 0x00007fffefce7600 in ?? () from /lib64/libc.so.6 > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1474074 > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > --- > src/channel-port.c | 7 +++++-- > src/channel-webdav.c | 13 +++++++++++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/src/channel-port.c b/src/channel-port.c > index d922e4b..f9d12e8 100644 > --- a/src/channel-port.c > +++ b/src/channel-port.c > @@ -22,6 +22,8 @@ > #include "spice-channel-priv.h" > #include "spice-marshal.h" > > +static void port_set_opened(SpicePortChannel *self, gboolean opened); > + > /** > * SECTION:channel-port > * @short_description: private communication channel > @@ -116,10 +118,11 @@ static void spice_port_channel_finalize(GObject > *object) > > static void spice_port_channel_reset(SpiceChannel *channel, gboolean > migrating) > { > - SpicePortChannelPrivate *c = SPICE_PORT_CHANNEL(channel)->priv; > + SpicePortChannel *self = SPICE_PORT_CHANNEL(channel); > + SpicePortChannelPrivate *c = self->priv; > > g_clear_pointer(&c->name, g_free); > - c->opened = FALSE; > + port_set_opened(self, false); > So regarding migration, channel_reset is only used for SEMI_SEAMLESS, which can't guarantee the disruption of the stream by design (see https://cgit.freedesktop.org/spice/spice-protocol/commit/?id=3838ad140a046c4ddf42fef58c9727ecfdc09f9f for details). So it seems fine to reopen the port here. > SPICE_CHANNEL_CLASS(spice_port_channel_parent_class)->channel_reset(channel, > migrating); > } > diff --git a/src/channel-webdav.c b/src/channel-webdav.c > index 4a246b5..a38fa19 100644 > --- a/src/channel-webdav.c > +++ b/src/channel-webdav.c > @@ -514,6 +514,8 @@ static void port_event(SpiceWebdavChannel *self, gint > event) > g_cancellable_cancel(c->cancellable); > c->demuxing = FALSE; > g_hash_table_remove_all(c->clients); > + g_clear_pointer(&c->queue, output_queue_free); > + g_clear_object(&c->stream); > } > } > > @@ -563,7 +565,18 @@ static void spice_webdav_channel_dispose(GObject > *object) > > static void spice_webdav_channel_up(SpiceChannel *channel) > { > + SpiceWebdavChannelPrivate *c = SPICE_WEBDAV_CHANNEL(channel)->priv; > + > CHANNEL_DEBUG(channel, "up"); > + > + if (c->stream == NULL) { > + GOutputStream *ostream; > + > + /* In case the channel has been reset */ > + c->stream = spice_vmc_stream_new(SPICE_CHANNEL(channel)); > + ostream = g_io_stream_get_output_stream(G_IO_STREAM(c->stream)); > + c->queue = output_queue_new(ostream); Please drop the same code from init. > + } > } > With that, the patch looks correct to me, and testing is ok. I can still use the webdav channel after seamless migration, and doing a copy in a loop didn't show any error (more testing would be needed to check for corruption). ack _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel