Message ID | 20190307165824.18059-1-berrange@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2] io: fix handling of EOF / error conditions in websock GSource | expand |
ping On Thu, Mar 07, 2019 at 04:58:24PM +0000, Daniel P. Berrangé wrote: > We were never reporting the G_IO_HUP event when an end of file was hit > on the websocket channel. > > We also didn't report G_IO_ERR when we hit a fatal error processing the > websocket protocol. > > The latter in particular meant that the chardev code would not notice > when an eof/error was encountered on the websocket channel, unless the > guest OS happened to trigger a write operation. > > This meant that once the first client had quit, the chardev would never > listen to accept a new client. > > Fixes launchpad bug 1816819 > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > > Changed in v2: > - Fix typo > - Remove use of G_IO_IN at same time as HUP/ERR since. It is > not required since the websock source doesn't do a real > poll itself. > > io/channel-websock.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/io/channel-websock.c b/io/channel-websock.c > index dc43dc6bb9..77d30f0e4a 100644 > --- a/io/channel-websock.c > +++ b/io/channel-websock.c > @@ -1225,12 +1225,18 @@ qio_channel_websock_source_check(GSource *source) > QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source; > GIOCondition cond = 0; > > - if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) { > + if (wsource->wioc->rawinput.offset) { > cond |= G_IO_IN; > } > if (wsource->wioc->encoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) { > cond |= G_IO_OUT; > } > + if (wsource->wioc->io_eof) { > + cond |= G_IO_HUP; > + } > + if (wsource->wioc->io_err) { > + cond |= G_IO_ERR; > + } > > return cond & wsource->condition; > } > -- > 2.20.1 > Regards, Daniel
On Thu, Mar 07, 2019 at 04:58:24PM +0000, Daniel P. Berrangé wrote: > We were never reporting the G_IO_HUP event when an end of file was hit > on the websocket channel. > > We also didn't report G_IO_ERR when we hit a fatal error processing the > websocket protocol. > > The latter in particular meant that the chardev code would not notice > when an eof/error was encountered on the websocket channel, unless the > guest OS happened to trigger a write operation. > > This meant that once the first client had quit, the chardev would never > listen to accept a new client. > > Fixes launchpad bug 1816819 > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > > Changed in v2: > - Fix typo > - Remove use of G_IO_IN at same time as HUP/ERR since. It is > not required since the websock source doesn't do a real > poll itself. > > io/channel-websock.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) The patch LGTM. Acked-by: Stefano Garzarella <sgarzare@redhat.com> Thanks, Stefano
On Tue, Mar 19, 2019 at 12:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > ping > > On Thu, Mar 07, 2019 at 04:58:24PM +0000, Daniel P. Berrangé wrote: > > We were never reporting the G_IO_HUP event when an end of file was hit > > on the websocket channel. > > > > We also didn't report G_IO_ERR when we hit a fatal error processing the > > websocket protocol. > > > > The latter in particular meant that the chardev code would not notice > > when an eof/error was encountered on the websocket channel, unless the > > guest OS happened to trigger a write operation. > > > > This meant that once the first client had quit, the chardev would never > > listen to accept a new client. > > > > Fixes launchpad bug 1816819 > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > > > Changed in v2: > > - Fix typo > > - Remove use of G_IO_IN at same time as HUP/ERR since. It is > > not required since the websock source doesn't do a real > > poll itself. > > > > io/channel-websock.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/io/channel-websock.c b/io/channel-websock.c > > index dc43dc6bb9..77d30f0e4a 100644 > > --- a/io/channel-websock.c > > +++ b/io/channel-websock.c > > @@ -1225,12 +1225,18 @@ qio_channel_websock_source_check(GSource *source) > > QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source; > > GIOCondition cond = 0; > > > > - if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) { > > + if (wsource->wioc->rawinput.offset) { > > cond |= G_IO_IN; > > } > > if (wsource->wioc->encoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) { > > cond |= G_IO_OUT; > > } > > + if (wsource->wioc->io_eof) { > > + cond |= G_IO_HUP; > > + } > > + if (wsource->wioc->io_err) { > > + cond |= G_IO_ERR; > > + } > > > > return cond & wsource->condition; > > } > > -- > > 2.20.1 > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/io/channel-websock.c b/io/channel-websock.c index dc43dc6bb9..77d30f0e4a 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -1225,12 +1225,18 @@ qio_channel_websock_source_check(GSource *source) QIOChannelWebsockSource *wsource = (QIOChannelWebsockSource *)source; GIOCondition cond = 0; - if (wsource->wioc->rawinput.offset || wsource->wioc->io_eof) { + if (wsource->wioc->rawinput.offset) { cond |= G_IO_IN; } if (wsource->wioc->encoutput.offset < QIO_CHANNEL_WEBSOCK_MAX_BUFFER) { cond |= G_IO_OUT; } + if (wsource->wioc->io_eof) { + cond |= G_IO_HUP; + } + if (wsource->wioc->io_err) { + cond |= G_IO_ERR; + } return cond & wsource->condition; }
We were never reporting the G_IO_HUP event when an end of file was hit on the websocket channel. We also didn't report G_IO_ERR when we hit a fatal error processing the websocket protocol. The latter in particular meant that the chardev code would not notice when an eof/error was encountered on the websocket channel, unless the guest OS happened to trigger a write operation. This meant that once the first client had quit, the chardev would never listen to accept a new client. Fixes launchpad bug 1816819 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- Changed in v2: - Fix typo - Remove use of G_IO_IN at same time as HUP/ERR since. It is not required since the websock source doesn't do a real poll itself. io/channel-websock.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)