[Spice-devel] [PATCH] server/spicevmc: Don't destroy the rcc twice

[Alon Levy]

On Sat, Feb 18, 2012 at 04:04:04PM +0100, Hans de Goede wrote:
> spicevmc calls red_channel_client_destroy() on the rcc when it disconnects
> since we don't want to delay the destroy until the session gets closed as
> spicevmc channels can be opened, closed and opened again during a single
> session.
> 
> This causes red_channel_client_destroy() to get called twice, triggering
> an assert, when a connected channel gets destroyed.
> 
> This was fixed with commit ffc4de01e6f9ea0676f17b10e45a137d7e15d6ac for
> the case where: a spicevmc channel was open on client disconnected, and
> the main channel disconnect gets handled first.
> 
> But the channel can also be destroyed when the chardev gets unregistered
> with the spice-server. This path still triggers the assert.
> 
> This patch fixes this by adding a destroying flag to the rcc struct, and
> also moves the previous fix over to the same, more clean, method of
> detecting this special case.

ACK.

> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  server/red_channel.c |    2 ++
>  server/red_channel.h |    1 +
>  server/spicevmc.c    |    6 +++---
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/server/red_channel.c b/server/red_channel.c
> index ec02018..6296dc9 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -780,6 +780,7 @@ void red_channel_set_data(RedChannel *channel, void *data)
>  
>  void red_channel_client_destroy(RedChannelClient *rcc)
>  {
> +    rcc->destroying = 1;
>      if (red_channel_client_is_connected(rcc)) {
>          red_channel_client_disconnect(rcc);
>      }
> @@ -1439,6 +1440,7 @@ void red_client_destroy(RedClient *client)
>          // some channels may be in other threads, so disconnection
>          // is not synchronous.
>          rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
> +        rcc->destroying = 1;
>          // some channels may be in other threads. However we currently
>          // assume disconnect is synchronous (we changed the dispatcher
>          // to wait for disconnection)
> diff --git a/server/red_channel.h b/server/red_channel.h
> index 045bfd4..543aec1 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -255,6 +255,7 @@ struct RedChannelClient {
>  
>      RedChannelCapabilities remote_caps;
>      int is_mini_header;
> +    int destroying;
>  };
>  
>  struct RedChannel {
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 5cdc513..30aaf2f 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -116,9 +116,9 @@ static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
>      sin = state->chardev_sin;
>      sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);
>  
> -    /* Don't destroy the rcc if the entire client is disconnecting, as then
> -       red_client_destroy will already do this! */
> -    if (!rcc->client->disconnecting)
> +    /* Don't destroy the rcc if it is already being destroyed, as then
> +       red_client_destroy/red_channel_client_destroy will already do this! */
> +    if (!rcc->destroying)
>          red_channel_client_destroy(rcc);
>  
>      state->rcc = NULL;
> -- 
> 1.7.7.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel