[Spice-commits] 2 commits - src/channel-usbredir.c src/spice-session.c

[GitLab Mirror]

 src/channel-usbredir.c |    1 -
 src/spice-session.c    |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

New commits:
commit 1c08a1171c3815f89be84b3678657fc9c50cd140
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed May 30 16:40:12 2018 -0500

    Add SpiceSession::disconnected signal
    
    Previously, an application was required to determine whether a session
    had been disconnected by listening for the SpiceSession::channel-destroy
    signal for each individual channel. The Application had to keep track of
    whether all channels had been destroyed.
    
    An additional difficulty is that when a SpiceSession::channel-destroy
    signal is emitted, the channel object has not been freed yet, so any
    shutdown/de-allocation actions have not yet happened. This became
    significant since virt-viewer exits the application in response to the
    last 'channel-destroy' signal, which means that the channel objects are
    never properly freed. This is particularly problematic for the usbredir
    channel, which might need to disconnect USB devices asynchronously, and
    if the application exits before that process has completed, the USB
    devices may not be available on the client machine.
    
    With this patch, the SpiceSession still emits the 'channel-destroy'
    signal for each channel when it is disconnected. But it also internally
    tracks which channels have been destroyed, and waits for them to be
    freed. When all channels are freed, it emits an additional
    'disconnected' signal.  An application can be confident that all
    channels have been freed by this point, and the application can exit
    safely.
    
    Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at gmail.com>

diff --git a/src/spice-session.c b/src/spice-session.c
index 57acc63..844d9d6 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -94,6 +94,7 @@ struct _SpiceSessionPrivate {
     int               protocol;
     SpiceChannel      *cmain; /* weak reference */
     Ring              channels;
+    guint             channels_destroying;
     guint32           mm_time;
     gboolean          client_provided_sockets;
     guint64           mm_time_at_clock;
@@ -210,6 +211,7 @@ enum {
     SPICE_SESSION_CHANNEL_NEW,
     SPICE_SESSION_CHANNEL_DESTROY,
     SPICE_SESSION_MM_TIME_RESET,
+    SPICE_SESSION_DISCONNECTED,
     SPICE_SESSION_LAST_SIGNAL,
 };
 
@@ -343,6 +345,8 @@ spice_session_dispose(GObject *gobject)
     g_warn_if_fail(s->migration_left == NULL);
     g_warn_if_fail(s->after_main_init == 0);
     g_warn_if_fail(s->disconnecting == 0);
+    g_warn_if_fail(s->channels_destroying == 0);
+    g_warn_if_fail(ring_is_empty(&s->channels));
 
     g_clear_object(&s->audio_manager);
     g_clear_object(&s->usb_manager);
@@ -1314,6 +1318,23 @@ static void spice_session_class_init(SpiceSessionClass *klass)
                      SPICE_TYPE_CHANNEL);
 
     /**
+     * SpiceSession::disconnected:
+     * @session: the session that emitted the signal
+     *
+     * The #SpiceSession::disconnected signal is emitted when all channels have been destroyed.
+     * Since: 0.35
+     **/
+    signals[SPICE_SESSION_DISCONNECTED] =
+        g_signal_new("disconnected",
+                     G_OBJECT_CLASS_TYPE(gobject_class),
+                     G_SIGNAL_RUN_FIRST,
+                     0, NULL, NULL,
+                     g_cclosure_marshal_VOID__VOID,
+                     G_TYPE_NONE,
+                     0,
+                     NULL);
+
+    /**
      * SpiceSession::mm-time-reset:
      * @session: the session that emitted the signal
      *
@@ -2269,6 +2290,17 @@ void spice_session_channel_new(SpiceSession *session, SpiceChannel *channel)
     g_signal_emit(session, signals[SPICE_SESSION_CHANNEL_NEW], 0, channel);
 }
 
+static void channel_finally_destroyed(gpointer data, GObject *channel)
+{
+    SpiceSession *session = SPICE_SESSION(data);
+    SpiceSessionPrivate *s = session->priv;
+    s->channels_destroying--;
+    if (ring_is_empty(&s->channels) && (s->channels_destroying == 0)) {
+        g_signal_emit(session, signals[SPICE_SESSION_DISCONNECTED], 0);
+    }
+    g_object_unref(session);
+}
+
 static void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
 {
     g_return_if_fail(SPICE_IS_SESSION(session));
@@ -2302,6 +2334,12 @@ static void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *c
 
     g_clear_object(&channel->priv->session);
     spice_channel_disconnect(channel, SPICE_CHANNEL_NONE);
+
+    /* Wait until the channel is properly freed so that we can emit a
+     * 'disconnected' signal */
+    s->channels_destroying++;
+    g_object_weak_ref(G_OBJECT(channel), channel_finally_destroyed, g_object_ref(session));
+
     g_object_unref(channel);
 }
 
commit 4b730b1aa8d6d606979a653eeddbb6552af6128b
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed May 30 16:36:18 2018 -0500

    Don't unref GAsyncResult in callback
    
    A GAsyncReadCallback function should not unref the GAsyncResult passed
    to it as an argument since it does not hold a reference. This could
    cause a double-free error (or a glib warning that it is not a valid
    GObject)..
    
    Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
    Acked-by: Marc-André Lureau <marcandre.lureau at gmail.com>

diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 0cc5630..b7445b0 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -151,7 +151,6 @@ static void _channel_reset_cb(GObject *gobject,
     SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(spice_channel, migrating);
 
     spice_usbredir_channel_disconnect_device_finish(channel, result, &err);
-    g_object_unref(result);
 }
 
 static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating)