On Tue, May 15, 2012 at 11:57:05AM +0300, Yonit Halperin wrote:
> 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.
Why can't we have GlzDrawable reference Drawable?
>
> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel