On Wed, Jun 6, 2018 at 9:39 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
> 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.
> ---
> Changes in v4:
> - add ring_is_empty() check to channel_finally_destroyed()
ack
>
>
> src/spice-session.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> 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);
> @@ -1313,6 +1317,23 @@ static void spice_session_class_init(SpiceSessionClass *klass)
> 1,
> 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);
> }
>
> --
> 2.14.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel