io: Always remove an old channel watch before adding a new one.

[Qemu-devel] [PATCH] io: Always remove an old channel watch before adding a new one.

Posted by Brandon Carpenter 8 years, 3 months ago

Also set saved handle to zero when removing without adding a new watch.

Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
---
 ui/vnc-auth-vencrypt.c | 3 +++
 ui/vnc-ws.c            | 6 ++++++
 ui/vnc.c               | 4 ++++
 3 files changed, 13 insertions(+)

diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index ffaab57550..c3eece4fa7 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -77,6 +77,9 @@ static void vnc_tls_handshake_done(QIOTask *task,
         vnc_client_error(vs);
         error_free(err);
     } else {
+        if (vs->ioc_tag) {
+            g_source_remove(vs->ioc_tag);
+        }
         vs->ioc_tag = qio_channel_add_watch(
             vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
         start_auth_vencrypt_subauth(vs);
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index f530cd5474..eaf309553c 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -36,6 +36,9 @@ static void vncws_tls_handshake_done(QIOTask *task,
         error_free(err);
     } else {
         VNC_DEBUG("TLS handshake complete, starting websocket handshake\n");
+        if (vs->ioc_tag) {
+            g_source_remove(vs->ioc_tag);
+        }
         vs->ioc_tag = qio_channel_add_watch(
             QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
     }
@@ -97,6 +100,9 @@ static void vncws_handshake_done(QIOTask *task,
     } else {
         VNC_DEBUG("Websock handshake complete, starting VNC protocol\n");
         vnc_start_protocol(vs);
+        if (vs->ioc_tag) {
+            g_source_remove(vs->ioc_tag);
+        }
         vs->ioc_tag = qio_channel_add_watch(
             vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
     }
diff --git a/ui/vnc.c b/ui/vnc.c
index eb91559b6b..ec86d6ad97 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1121,6 +1121,7 @@ static void vnc_disconnect_start(VncState *vs)
     vnc_set_share_mode(vs, VNC_SHARE_MODE_DISCONNECTED);
     if (vs->ioc_tag) {
         g_source_remove(vs->ioc_tag);
+        vs->ioc_tag = 0;
     }
     qio_channel_close(vs->ioc, NULL);
     vs->disconnecting = TRUE;
@@ -2931,6 +2932,9 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
     VNC_DEBUG("New client on socket %p\n", vs->sioc);
     update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
     qio_channel_set_blocking(vs->ioc, false, NULL);
+    if (vs->ioc_tag) {
+        g_source_remove(vs->ioc_tag);
+    }
     if (websocket) {
         vs->websocket = 1;
         if (vd->tlscreds) {
-- 
2.13.3


-- 


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

Re: [Qemu-devel] [PATCH] io: Always remove an old channel watch before adding a new one.

Posted by Paolo Bonzini 8 years, 3 months ago

On 24/07/2017 20:15, Brandon Carpenter wrote:
> Also set saved handle to zero when removing without adding a new watch.
> 
> Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
> ---
>  ui/vnc-auth-vencrypt.c | 3 +++
>  ui/vnc-ws.c            | 6 ++++++
>  ui/vnc.c               | 4 ++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
> index ffaab57550..c3eece4fa7 100644
> --- a/ui/vnc-auth-vencrypt.c
> +++ b/ui/vnc-auth-vencrypt.c
> @@ -77,6 +77,9 @@ static void vnc_tls_handshake_done(QIOTask *task,
>          vnc_client_error(vs);
>          error_free(err);
>      } else {
> +        if (vs->ioc_tag) {
> +            g_source_remove(vs->ioc_tag);
> +        }
>          vs->ioc_tag = qio_channel_add_watch(
>              vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
>          start_auth_vencrypt_subauth(vs);
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index f530cd5474..eaf309553c 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -36,6 +36,9 @@ static void vncws_tls_handshake_done(QIOTask *task,
>          error_free(err);
>      } else {
>          VNC_DEBUG("TLS handshake complete, starting websocket handshake\n");
> +        if (vs->ioc_tag) {
> +            g_source_remove(vs->ioc_tag);
> +        }
>          vs->ioc_tag = qio_channel_add_watch(
>              QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
>      }
> @@ -97,6 +100,9 @@ static void vncws_handshake_done(QIOTask *task,
>      } else {
>          VNC_DEBUG("Websock handshake complete, starting VNC protocol\n");
>          vnc_start_protocol(vs);
> +        if (vs->ioc_tag) {
> +            g_source_remove(vs->ioc_tag);
> +        }
>          vs->ioc_tag = qio_channel_add_watch(
>              vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
>      }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index eb91559b6b..ec86d6ad97 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1121,6 +1121,7 @@ static void vnc_disconnect_start(VncState *vs)
>      vnc_set_share_mode(vs, VNC_SHARE_MODE_DISCONNECTED);
>      if (vs->ioc_tag) {
>          g_source_remove(vs->ioc_tag);
> +        vs->ioc_tag = 0;
>      }
>      qio_channel_close(vs->ioc, NULL);
>      vs->disconnecting = TRUE;
> @@ -2931,6 +2932,9 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
>      VNC_DEBUG("New client on socket %p\n", vs->sioc);
>      update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_BASE);
>      qio_channel_set_blocking(vs->ioc, false, NULL);
> +    if (vs->ioc_tag) {
> +        g_source_remove(vs->ioc_tag);
> +    }
>      if (websocket) {
>          vs->websocket = 1;
>          if (vd->tlscreds) {
> 


Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH] io: Always remove an old channel watch before adding a new one.

Posted by Daniel P. Berrange 8 years, 3 months ago

On Mon, Jul 24, 2017 at 11:15:44AM -0700, Brandon Carpenter wrote:
> Also set saved handle to zero when removing without adding a new watch.
> 
> Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
> ---
>  ui/vnc-auth-vencrypt.c | 3 +++
>  ui/vnc-ws.c            | 6 ++++++
>  ui/vnc.c               | 4 ++++
>  3 files changed, 13 insertions(+)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] io: Always remove an old channel watch before adding a new one.

Posted by Brandon Carpenter 8 years, 1 month ago

I haven't seen this patch hit master yet and am about to submit a patch 
set that is dependent on this one because it triggers the bug fixed by 
this patch, causing a segmentation fault. Is it preferred that I 
include this patch in that series with the Reviewed-by: tags or to just 
reference this patch in the cover letter?

Thanks.
--
Brandon Carpenter | Software Engineer
Cypherpath, Inc.
400 Columbia Point Drive Ste 101 | Richland, Washington USA
Office: (650) 713-3060

On Tue, Jul 25, 2017 at 1:36 AM, Daniel P. Berrange 
<berrange@redhat.com> wrote:
> On Mon, Jul 24, 2017 at 11:15:44AM -0700, Brandon Carpenter wrote:
>>  Also set saved handle to zero when removing without adding a new 
>> watch.
>> 
>>  Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
>>  ---
>>   ui/vnc-auth-vencrypt.c | 3 +++
>>   ui/vnc-ws.c            | 6 ++++++
>>   ui/vnc.c               | 4 ++++
>>   3 files changed, 13 insertions(+)
> 
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> 
> 
> 
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    
> https://www.instagram.com/dberrange :|

-- 

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

Re: [Qemu-devel] [PATCH] io: Always remove an old channel watch before adding a new one.

Posted by Daniel P. Berrange 8 years, 1 month ago

On Fri, Sep 08, 2017 at 09:18:04AM -0700, Brandon Carpenter wrote:
> I haven't seen this patch hit master yet and am about to submit a patch set
> that is dependent on this one because it triggers the bug fixed by this
> patch, causing a segmentation fault. Is it preferred that I include this
> patch in that series with the Reviewed-by: tags or to just reference this
> patch in the cover letter?

Yes, if you re-post the patch as part of a larger just, add any
Reviewed-by tags to the commit message when you re-post.

> On Tue, Jul 25, 2017 at 1:36 AM, Daniel P. Berrange <berrange@redhat.com>
> wrote:
> > On Mon, Jul 24, 2017 at 11:15:44AM -0700, Brandon Carpenter wrote:
> > >  Also set saved handle to zero when removing without adding a new
> > > watch.
> > > 
> > >  Signed-off-by: Brandon Carpenter <brandon.carpenter@cypherpath.com>
> > >  ---
> > >   ui/vnc-auth-vencrypt.c | 3 +++
> > >   ui/vnc-ws.c            | 6 ++++++
> > >   ui/vnc.c               | 4 ++++
> > >   3 files changed, 13 insertions(+)
> > 
> > Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> > 
> > 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|