[Spice-devel] [PATCH 3/3] server/red_worker: fix possible leak of self_bitmap

[Alon Levy]

After the previous patch moving self_bitmap freeing inside red_drawable
ref count, we have a possible self_bitmap leak:

red_process_commands
 red_get_drawable | red_drawable #1, red_drawable->self_bitmap == 1
  red_process_drawable | rd #2, d #1, d->self_bitmap != NULL
   release_drawable | rd #1, d# = 0, try to release self_bitmap, but
                        blocked by rd #1
  put_red_drawable | rd #0

This patch moves the call to release_drawable after the call to
put_red_drawable, thereby fixing the above situation.
---
 server/red_worker.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index e1c86fa..1adf4c9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3867,8 +3867,8 @@ static inline void red_inc_surfaces_drawable_dependencies(RedWorker *worker, Dra
     }
 }
 
-static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable,
-                                        uint32_t group_id)
+static inline Drawable *red_process_drawable(RedWorker *worker, RedDrawable *drawable,
+                                             uint32_t group_id)
 {
     int surface_id;
     Drawable *item = get_drawable(worker, drawable->effect, drawable, group_id);
@@ -3931,7 +3931,7 @@ static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable
 #endif
     }
 cleanup:
-    release_drawable(worker, item);
+    return item;
 }
 
 static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uint32_t width,
@@ -4844,14 +4844,16 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
         switch (ext_cmd.cmd.type) {
         case QXL_CMD_DRAW: {
             RedDrawable *red_drawable = red_drawable_new(); // returns with 1 ref
+            Drawable *drawable;
 
             if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
                                  red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
                 break;
             }
-            red_process_drawable(worker, red_drawable, ext_cmd.group_id);
-            // release the red_drawable
+            drawable = red_process_drawable(worker, red_drawable, ext_cmd.group_id);
+            // release red_drawable first, it will not die because drawable holds a reference on it.
             put_red_drawable(worker, red_drawable, ext_cmd.group_id, NULL);
+            release_drawable(worker, drawable);
             break;
         }
         case QXL_CMD_UPDATE: {
-- 
1.7.10.1