| Message ID | 20190220182543.10623-1-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io: fix handling of EOF / error conditions in websock GSource | expand |
Hi Daniel, On 2/20/19 7:25 PM, 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 ddn't report G_IO_ERR when we hit a fatal error processing the "didn't report" > 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> > --- > 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..fc4c3dcaa5 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_IN | G_IO_HUP; Shouldn't this be: cond |= G_IO_HUP; > + } > + if (wsource->wioc->io_err) { > + cond |= G_IO_IN | G_IO_ERR; Ditto: cond |= G_IO_ERR; > + } > > return cond & wsource->condition; > } >
On Wed, Feb 20, 2019 at 08:52:34PM +0100, Philippe Mathieu-Daudé wrote: > Hi Daniel, > > On 2/20/19 7:25 PM, 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 ddn't report G_IO_ERR when we hit a fatal error processing the > > "didn't report" > > > 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> > > --- > > 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..fc4c3dcaa5 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_IN | G_IO_HUP; > > Shouldn't this be: > > cond |= G_IO_HUP; With regular poll() on FD I believe you typically get POLLIN | POLLHUP as a pair. > > > + } > > + if (wsource->wioc->io_err) { > > + cond |= G_IO_IN | G_IO_ERR; > > Ditto: > > cond |= G_IO_ERR; And I just followed the same practice here. > > + } > > > > return cond & wsource->condition; Regards, Daniel
diff --git a/io/channel-websock.c b/io/channel-websock.c index dc43dc6bb9..fc4c3dcaa5 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_IN | G_IO_HUP; + } + if (wsource->wioc->io_err) { + cond |= G_IO_IN | 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 ddn'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> --- io/channel-websock.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)