[Spice-devel] [spice-gtk] cache: Fix OOM due bad refcount of image

Victor Toso victortoso at redhat.com
Fri Jul 20 09:56:29 UTC 2018


From: 乐义华 <yueyihua at os-easy.com>

The image in cache was not being freed properly leading the client to
crash with OOM after sometime.

When image flag is SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME, and if found it
in hash table, we will increase the refcount but this image will only
be destroyed, so the cache will usage more and more memory. In this
circumstance, we should not increase refcount.

Fixes: https://gitlab.freedesktop.org/spice/spice-gtk/issues/72

Signed-off-by: 乐义华 <yueyihua at os-easy.com>
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 src/channel-display.c     |  5 ++++-
 src/spice-channel-cache.h | 17 +++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/channel-display.c b/src/channel-display.c
index 54f40ba..7c663cb 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);
 }
 
 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..fc25c35 100644
--- a/src/spice-channel-cache.h
+++ b/src/spice-channel-cache.h
@@ -101,6 +101,23 @@ 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);
+
+    // If image is currently in the table consider its reference count before replacing it
+    if (cache->ref_counted) {
+        display_cache_item *current_item;
+        gpointer current_image;
+        if (g_hash_table_lookup_extended(cache->table, &id, (gpointer*) &current_item,
+                                         &current_image)) {
+            item->ref_count = current_item->ref_count;
+        }
+    }
+    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);
-- 
2.17.1



More information about the Spice-devel mailing list