[Spice-devel] [PATCH spice-server] red-channel: unregister channel in red_channel_destroy

[Jonathon Jongsma]

Seems like an improvement.

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Fri, 2017-08-18 at 12:14 +0100, Frediano Ziglio wrote:
> Mostly of red_channel_destroy calls were preceded by
> a call to unregister the channel.
> The only exception was the main channel as this channel is
> always present and its initialisation is a bit different.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red-channel.c | 3 +++
>  server/red-worker.c  | 6 ------
>  server/reds.c        | 7 +------
>  server/sound.c       | 2 --
>  server/spicevmc.c    | 8 +-------
>  5 files changed, 5 insertions(+), 21 deletions(-)
> 
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 9bd98234..a676ffe9 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -402,6 +402,9 @@ void red_channel_destroy(RedChannel *channel)
>          return;
>      }
>  
> +    // prevent future connection
> +    reds_unregister_channel(channel->priv->reds, channel);
> +
>      g_list_foreach(channel->priv->clients,
> (GFunc)red_channel_client_destroy, NULL);
>      g_object_unref(channel);
>  }
> diff --git a/server/red-worker.c b/server/red-worker.c
> index f747e128..8e165877 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1431,12 +1431,6 @@ static void
> red_worker_close_channel(RedChannel *channel)
>   */
>  void red_worker_free(RedWorker *worker)
>  {
> -    RedsState *reds = red_qxl_get_server(worker->qxl->st);
> -
> -    /* prevent any possible future attempt to connect to new clients
> */
> -    reds_unregister_channel(reds, RED_CHANNEL(worker-
> >cursor_channel));
> -    reds_unregister_channel(reds, RED_CHANNEL(worker-
> >display_channel));
> -
>      pthread_join(worker->thread, NULL);
>  
>      red_worker_close_channel(RED_CHANNEL(worker->cursor_channel));
> diff --git a/server/reds.c b/server/reds.c
> index 8abfbda5..55e362c7 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -391,11 +391,7 @@ void reds_register_channel(RedsState *reds,
> RedChannel *channel)
>  
>  void reds_unregister_channel(RedsState *reds, RedChannel *channel)
>  {
> -    if (g_list_find(reds->channels, channel)) {
> -        reds->channels = g_list_remove(reds->channels, channel);
> -    } else {
> -        spice_warning("not found");
> -    }
> +    reds->channels = g_list_remove(reds->channels, channel);
>  }
>  
>  RedChannel *reds_find_channel(RedsState *reds, uint32_t type,
> uint32_t id)
> @@ -3712,7 +3708,6 @@ SPICE_GNUC_VISIBLE void
> spice_server_destroy(SpiceServer *reds)
>      g_list_free_full(reds->qxl_instances,
> (GDestroyNotify)red_qxl_destroy);
>  
>      if (reds->inputs_channel) {
> -        reds_unregister_channel(reds, RED_CHANNEL(reds-
> >inputs_channel));
>          red_channel_destroy(RED_CHANNEL(reds->inputs_channel));
>      }
>      if (reds->main_channel) {
> diff --git a/server/sound.c b/server/sound.c
> index be7e6076..54b89713 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1428,9 +1428,7 @@ static void snd_detach_common(SndChannel
> *channel)
>      if (!channel) {
>          return;
>      }
> -    RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
> -    reds_unregister_channel(reds, RED_CHANNEL(channel));
>      red_channel_destroy(RED_CHANNEL(channel));
>  }
>  
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 34d5c6e4..27e95947 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -897,17 +897,11 @@ red_char_device_spicevmc_dispose(GObject
> *object)
>      RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
>  
>      if (self->channel) {
> -        RedChannel *channel = RED_CHANNEL(self->channel);
> -        RedsState *reds =
> red_char_device_get_server(RED_CHAR_DEVICE(self));
> -
>          // prevent possible recursive calls
>          self->channel->chardev = NULL;
>  
> -        // prevent future connection
> -        reds_unregister_channel(reds, channel);
> -
>          // close all current connections and drop the reference
> -        red_channel_destroy(channel);
> +        red_channel_destroy(RED_CHANNEL(self->channel));
>          self->channel = NULL;
>      }
>      G_OBJECT_CLASS(red_char_device_spicevmc_parent_class)-
> >dispose(object);