[Spice-devel] [spice-gtk] If replace me, should nod do refcount plus one

[Frediano Ziglio]

> 
> From: 乐义华 <yueyihua at os-easy.com>
> 
> https://gitlab.freedesktop.org/spice/spice-gtk/issues/72
> ---
>  src/channel-display.c     | 5 ++++-
>  src/spice-channel-cache.h | 8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 44ba043..138cd8c 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -788,7 +788,10 @@ static void image_put_lossy(SpiceImageCache *cache,
> uint64_t id,
>  static void image_replace_lossy(SpiceImageCache *cache, uint64_t id,
>                                  pixman_image_t *surface)
>  {
> -    image_put(cache, id, surface);
> +    SpiceDisplayChannelPrivate *c =
> +        SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate, image_cache);
> +
> +    cache_replace_lossy(c->images, id, pixman_image_ref(surface), FALSE);
>  }
>  

Maybe just add a cache_remove before image_put?
image_put always uses a cache_add which add a reference so creating the leak
as apparently image_replace_lossy is supposed to replace, not add a reference
to the cache.
In theory if the reference count of that item is already, for instance, 2
calling g_hash_table_replace directly will ignore the reference counting.
Adding the call to cache_remove and then image_put will keep the
reference count to 2.

>  static pixman_image_t* image_get_lossless(SpiceImageCache *cache, uint64_t
>  id)
> diff --git a/src/spice-channel-cache.h b/src/spice-channel-cache.h
> index 75cc2cd..1df8f34 100644
> --- a/src/spice-channel-cache.h
> +++ b/src/spice-channel-cache.h
> @@ -101,6 +101,14 @@ static inline void cache_add_lossy(display_cache *cache,
> uint64_t id,
>      g_hash_table_replace(cache->table, item, value);
>  }
>  
> +static inline void cache_replace_lossy(display_cache *cache, uint64_t id,
> +                                   gpointer value, gboolean lossy)
> +{
> +    display_cache_item *item = cache_item_new(id, lossy);
> +
> +    g_hash_table_replace(cache->table, item, value);
> +}
> +
>  static inline void cache_add(display_cache *cache, uint64_t id, gpointer
>  value)
>  {
>      cache_add_lossy(cache, id, value, FALSE);

Frediano