>
> No need to re-implement refcounting in this subclass. However, I needed
> to add a new 'dcc' member to UpgradeItem to be able to unref properly.
> ---
> Change since last patch: take ref on dcc
>
> server/dcc.c | 18 +-----------------
> server/display-channel.c | 4 +---
> server/display-channel.h | 2 +-
> server/stream.c | 18 ++++++++++++++++--
> 4 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index 91c3f82..5193c17 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1591,29 +1591,15 @@ int dcc_handle_migrate_data(DisplayChannelClient
> *dcc, uint32_t size, void *mess
> return TRUE;
> }
>
> -static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
> -{
> - if (--item->refs != 0)
> - return;
> -
> - display_channel_drawable_unref(display, item->drawable);
> - free(item->rects);
> - free(item);
> -}
> -
> static void release_item_after_push(DisplayChannelClient *dcc, PipeItem
> *item)
> {
> - DisplayChannel *display = DCC_TO_DC(dcc);
> -
> switch (item->type) {
> case PIPE_ITEM_TYPE_DRAW:
> case PIPE_ITEM_TYPE_IMAGE:
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> case PIPE_ITEM_TYPE_MONITORS_CONFIG:
> - pipe_item_unref(item);
> - break;
> case PIPE_ITEM_TYPE_UPGRADE:
> - upgrade_item_unref(display, (UpgradeItem *)item);
> + pipe_item_unref(item);
> break;
> case PIPE_ITEM_TYPE_GL_SCANOUT:
> case PIPE_ITEM_TYPE_GL_DRAW:
> @@ -1651,8 +1637,6 @@ static void
> release_item_before_push(DisplayChannelClient *dcc, PipeItem *item)
> }
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> case PIPE_ITEM_TYPE_UPGRADE:
> - upgrade_item_unref(display, (UpgradeItem *)item);
> - break;
> case PIPE_ITEM_TYPE_IMAGE:
> case PIPE_ITEM_TYPE_MONITORS_CONFIG:
> pipe_item_unref(item);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 88dbc74..98b4a43 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1973,10 +1973,8 @@ static void hold_item(RedChannelClient *rcc, PipeItem
> *item)
> case PIPE_ITEM_TYPE_DRAW:
> case PIPE_ITEM_TYPE_IMAGE:
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> - pipe_item_ref(item);
> - break;
> case PIPE_ITEM_TYPE_UPGRADE:
> - ((UpgradeItem *)item)->refs++;
> + pipe_item_ref(item);
> break;
> default:
> spice_warn_if_reached();
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 6b053de..8944f06 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -244,7 +244,7 @@ typedef struct SurfaceDestroyItem {
>
> typedef struct UpgradeItem {
> PipeItem base;
> - int refs;
> + DisplayChannelClient *dcc;
Here a display channel is enough I think.
> Drawable *drawable;
> SpiceClipRects *rects;
> } UpgradeItem;
> diff --git a/server/stream.c b/server/stream.c
> index ae37a62..80cfe2e 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -758,6 +758,18 @@ void stream_agent_stop(StreamAgent *agent)
> }
> }
>
> +static void upgrade_item_free(UpgradeItem *item)
> +{
> + g_return_if_fail(item != NULL);
> + DisplayChannel *display = DCC_TO_DC(item->dcc);
> + g_return_if_fail(item->base.refcount != 0);
> +
> + display_channel_drawable_unref(display, item->drawable);
I'm still pondering this.
My initial question was mainly a question (even to me) than a
proposal. Here the problem is that this can create a circular
reference counting. The problem is quite hidden by the fact that
we don't still have a way to free part of our stuff (DisplayChannel
is one). This make hard to verify this, I should have pushed the
inclusion of my cleanup branch when was refused.
Jonathon... did you test for leaks?
> + free(item->rects);
> + red_channel_client_unref(RED_CHANNEL_CLIENT(item->dcc));
> + free(item);
> +}
> +
> /*
> * after dcc_detach_stream_gracefully is called for all the display channel
> clients,
> * detach_stream should be called. See comment (1).
> @@ -798,10 +810,12 @@ static void
> dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
> rect_debug(&stream->current->red_drawable->bbox);
> rcc = RED_CHANNEL_CLIENT(dcc);
> upgrade_item = spice_new(UpgradeItem, 1);
> - upgrade_item->refs = 1;
> - pipe_item_init(&upgrade_item->base, PIPE_ITEM_TYPE_UPGRADE);
> + pipe_item_init_full(&upgrade_item->base, PIPE_ITEM_TYPE_UPGRADE,
> + (GDestroyNotify)upgrade_item_free);
> upgrade_item->drawable = stream->current;
> upgrade_item->drawable->refs++;
> + upgrade_item->dcc = dcc;
> + red_channel_client_ref(RED_CHANNEL_CLIENT(upgrade_item->dcc));
> n_rects =
> pixman_region32_n_rects(&upgrade_item->drawable->tree_item.base.rgn);
> upgrade_item->rects = spice_malloc_n_m(n_rects, sizeof(SpiceRect),
> sizeof(SpiceClipRects));
> upgrade_item->rects->num_rects = n_rects;
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel