Hi,
Instead of this patch series and the previous "self_bitmap" patch, I
think we should do the following:
Both GLZDrawable and Drawable share references to RedDrawable and
self_bitmap, and self_bitmap life time is equal to RedDrawable's one.
So we need to have a new type which warps RedDrawable and self_bitmap,
and move the ref count from RedDrawable to this new type.
Then, Drawable and GlzDrawable will just hold a reference to
RedDrawableWrapper, instead of two references to RedDrawable and
self_bitmap, and they will just decrease the reference to
RedDrawableWrapper when they are released.
Thanks,
Yonit.
On 05/14/2012 03:32 PM, Alon Levy wrote:
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: {
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
