From a3b0413fc28ee1f9c302cf98925b8b1832aeb970 Mon Sep 17 00:00:00 2001

From: Jonathon Jongsma <jjongsma@redhat.com>

Date: Tue, 29 Jan 2019 14:47:51 -0600

Subject: [PATCH] Fix a regression when initial connection fails

Due to changes in commit 65ef66e4, when the initial connection fails,

virt-viewer just sat quietly and didn't indicate what was wrong. It also

did not exit as it did before.  This is because we were using

virt_viewer_session_spice_channel_destroy() incorrectly. This function

was intended to be a callback that is called to clean up the VV session

when the SpiceSession tells us that a channel has been destroyed. It

does not actually destroy the channel, it only cleans up references to

that channel within virt-viewer. After calling this function, the

channel is not affected in any way. If the channel object was valid

before calling the function, it will be valid and unchanged after

calling the function as well.

The problem is that before commit 65ef66e4, this function

(_channel_destroy()) also had a side-effect of emitting a signal that

made us think that the SpiceSession was disconnected when it was not.

The application responded to this signal by exiting even though the

session was not properly disconnected and cleaned up.

We now no longer exit the application until the SpiceSession is properly

disconnected and cleaned up. So we need to make sure that this happens

when our initial connection fails. Therefore, when the main channel

receives an error channel-event, we should not call

virt_viewer_session_spice_channel_destroy(). This function should only

be called when a channel has actually been destroyed, and the channel is

not destroyed at this point.  We should instead explicitly disconnect

the session, which will result in the channels being destroyed properly.

After the session destroys all of the channels, the 'channel-destroy' signal

will be emitted by SpiceSession, so the _channel_destroy() function will

eventually get called by the signal handler.

To make the proper use of the function more obvious, I also changed the

function name from _channel_destroy() to _channel_destroyed() and added

a comment.

Fixes: rhbz#1666869

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>

Acked-by: Christophe Fergeau <cfergeau@redhat.com>

(cherry picked from commit c2dabf0730e1601745d2cdfc28f59e65e17cdab1)

---

 src/virt-viewer-session-spice.c | 19 ++++++++++---------

 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c

index d479a77..b664024 100644

--- a/src/virt-viewer-session-spice.c

+++ b/src/virt-viewer-session-spice.c

@@ -73,9 +73,9 @@ static void virt_viewer_session_spice_usb_device_selection(VirtViewerSession *se

 static void virt_viewer_session_spice_channel_new(SpiceSession *s,

                                                   SpiceChannel *channel,

                                                   VirtViewerSession *session);

-static void virt_viewer_session_spice_channel_destroy(SpiceSession *s,

-                                                      SpiceChannel *channel,

-                                                      VirtViewerSession *session);

+static void virt_viewer_session_spice_channel_destroyed(SpiceSession *s,

+                                                        SpiceChannel *channel,

+                                                        VirtViewerSession *session);

 static void virt_viewer_session_spice_session_disconnected(SpiceSession *s,

                                                            VirtViewerSessionSpice *session);

 static void virt_viewer_session_spice_smartcard_insert(VirtViewerSession *session);

@@ -401,7 +401,7 @@ create_spice_session(VirtViewerSessionSpice *self)

     virt_viewer_signal_connect_object(self->priv->session, "channel-new",

                                       G_CALLBACK(virt_viewer_session_spice_channel_new), self, 0);

     virt_viewer_signal_connect_object(self->priv->session, "channel-destroy",

-                                      G_CALLBACK(virt_viewer_session_spice_channel_destroy), self, 0);

+                                      G_CALLBACK(virt_viewer_session_spice_channel_destroyed), self, 0);

     virt_viewer_signal_connect_object(self->priv->session, "disconnected",

                                       G_CALLBACK(virt_viewer_session_spice_session_disconnected), self, 0);

@@ -770,14 +770,14 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel,

                 spice_session_connect(self->priv->session);

             }

         } else {

-            virt_viewer_session_spice_channel_destroy(NULL, channel, session);

+            spice_session_disconnect(self->priv->session);

         }

         break;

     }

     case SPICE_CHANNEL_ERROR_IO:

     case SPICE_CHANNEL_ERROR_LINK:

     case SPICE_CHANNEL_ERROR_TLS:

-        virt_viewer_session_spice_channel_destroy(NULL, channel, session);

+        spice_session_disconnect(self->priv->session);

         break;

     default:

         g_warning("unhandled spice main channel event: %d", event);

@@ -1104,10 +1104,11 @@ virt_viewer_session_spice_session_disconnected(G_GNUC_UNUSED SpiceSession *s,

     g_signal_emit_by_name(self, "session-disconnected", self->priv->disconnect_error);

 }

+/* called when the spice session indicates that a session has been destroyed */

 static void

-virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession *s,

-                                          SpiceChannel *channel,

-                                          VirtViewerSession *session)

+virt_viewer_session_spice_channel_destroyed(G_GNUC_UNUSED SpiceSession *s,

+                                            SpiceChannel *channel,

+                                            VirtViewerSession *session)

 {

     VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);

     int id;