It doesn't work well, I get a crash with pretty poor backtrace when
migrating with virt-manager (looks like an "extra" unref from python
GC).
On Thu, Nov 20, 2014 at 3:52 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> spice_session_disconnect gets rid of the reference it owns on
> the SpiceChannels listed in SpiceSession::channels. However, it does
> not remove them from that list, which would cause multiple calls to
> spice_session_disconnect to remove references it doesn't own, which
> will cause memory management issues later on.
>
> This patch removes the channels from SpiceSession::channels after they
> are unref'ed to ensure the reference SpiceSession owns will not be
> removed multiple times.
>
> It would be cleaner to clear the SpiceChannel::session when it's removed
> from the session, but client applications (virt-viewer)/other spice-gtk
> code do not deal well with SpiceChannel::session getting set to NULL
> when a disconnect event happens.
> ---
> Hey,
>
> I've experimented with that as well, and it seems that this patch would solve
> that issue with using spice-gtk from python/... (haven't tested virt-manager at
> all), and would cause less issues with existing code, what do you think?
>
> Christophe
>
>
> gtk/spice-session.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index c44a3e1..df71eb7 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -1650,8 +1650,12 @@ void spice_session_disconnect(SpiceSession *session)
> for (ring = ring_get_head(&s->channels); ring != NULL; ring = next) {
> next = ring_next(&s->channels, ring);
> item = SPICE_CONTAINEROF(ring, struct channel, link);
> - spice_channel_destroy(item->channel); /* /!\ item and channel are destroy() after this call */
> + ring_remove(&item->link);
> + spice_channel_disconnect(item->channel, SPICE_CHANNEL_NONE);
> + g_object_unref(item->channel);
> + free(item);
> }
> + g_warn_if_fail(ring_is_empty(&s->channels));
>
> s->connection_id = 0;
>
> @@ -1940,7 +1944,7 @@ void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
> {
> SpiceSessionPrivate *s = session->priv;
> struct channel *item = NULL;
> - RingItem *ring;
> + RingItem *ring = NULL;
>
> g_return_if_fail(s != NULL);
> g_return_if_fail(channel != NULL);
> @@ -1955,15 +1959,16 @@ void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel)
> break;
> }
>
> - g_return_if_fail(ring != NULL);
>
> if (channel == s->cmain) {
> CHANNEL_DEBUG(channel, "the session lost the main channel");
> s->cmain = NULL;
> }
>
> - ring_remove(&item->link);
> - free(item);
> + if (ring != NULL) {
> + ring_remove(&item->link);
> + free(item);
> + }
>
> g_signal_emit(session, signals[SPICE_SESSION_CHANNEL_DESTROY], 0, channel);
> }
> --
> 2.1.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel