> > > > > From: 乐义华 <yueyihua@xxxxxxxxxxx> > > > > 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); > Do we really need to use the reference counting on the image cache? Looking at protocol and spice-gtk source I don't see the reason. cache_add always call a g_hash_table_replace while cache_remove is only called when server requires specific invalidate. I cannot imagine a sequence of commands leading to a cache_remove that should not remove the id from the cache. After server send a CACHE_ME of an already existing ID all following images with that ID are referring to last image inserted with that ID so why server should send a INVAL_LIST with the ID referring to the old ID and not the new one? Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel