[Spice-devel] [PATCH 1/2] server: display_channel: releasing pipe items resources when the pipe is cleared (on disconnect)
Alon Levy
alevy at redhat.com
Tue Jul 5 07:05:46 PDT 2011
On Tue, Jul 05, 2011 at 03:50:40PM +0300, Yonit Halperin wrote:
> fixes "display_channel_release_item: panic: invalid item type"
>
ACK.
> Before changing the red_worker to use the red_channel interface, there
> was a devoted red_pipe_clear routine for the display channel and cursor channel.
> However, clearing the pipe in red_channel, uses the release_item callback
> the worker provided. This callback has handled only resources that need to be released
> after the pipe item was enqueued from the pipe, and only for pipe items that were set in
> red_channel_init_send_data.
> This fix changes the display channel release_item callback to handle all types of
> pipe items, and also handles differently pushed and non-pushed pipe items.
> ---
> server/red_worker.c | 99 ++++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 75 insertions(+), 24 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index c0a9760..fc4326c 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -934,6 +934,8 @@ static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBi
> static inline int _stride_is_extra(SpiceBitmap *bitmap);
> static void red_disconnect_cursor(RedChannel *channel);
> static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item);
> +static void display_channel_release_item_before_push(DisplayChannel *display_channel, PipeItem *item);
> +static void display_channel_release_item_after_push(DisplayChannel *display_channel, PipeItem *item);
>
> #ifdef DUMP_BITMAP
> static void dump_bitmap(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
> @@ -8012,89 +8014,75 @@ static void display_channel_send_item(RedChannel *base, PipeItem *pipe_item)
> {
> SpiceMarshaller *m = red_channel_get_marshaller(base);
> DisplayChannel *display_channel = (DisplayChannel *)red_ref_channel(base);
> - RedWorker *worker = display_channel->common.worker;
>
> red_display_reset_send_data(display_channel);
> switch (pipe_item->type) {
> case PIPE_ITEM_TYPE_DRAW: {
> Drawable *drawable = SPICE_CONTAINEROF(pipe_item, Drawable, pipe_item);
> marshall_qxl_drawable(display_channel, m, drawable);
> - release_drawable(worker, drawable);
> break;
> }
> case PIPE_ITEM_TYPE_INVAL_ONE:
> red_display_marshall_inval(display_channel, m, (CacheItem *)pipe_item);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_STREAM_CREATE: {
> StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent, create_item);
> red_display_marshall_stream_start(display_channel, m, agent);
> - red_display_release_stream(display_channel, agent);
> break;
> }
> case PIPE_ITEM_TYPE_STREAM_CLIP: {
> - StreamClipItem* clip_item = (StreamClipItem *)pipe_item;
> - red_display_marshall_stream_clip(display_channel, m, clip_item);
> - red_display_release_stream_clip(display_channel, clip_item);
> + red_display_marshall_stream_clip(display_channel, m, (StreamClipItem *)pipe_item);
> break;
> }
> case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent, destroy_item);
> red_display_marshall_stream_end(display_channel, m, agent);
> - red_display_release_stream(display_channel, agent);
> break;
> }
> case PIPE_ITEM_TYPE_UPGRADE:
> red_display_marshall_upgrade(display_channel, m, (UpgradeItem *)pipe_item);
> - release_upgrade_item(worker, (UpgradeItem *)pipe_item);
> break;
> case PIPE_ITEM_TYPE_VERB:
> display_marshall_verb(display_channel, ((VerbItem*)pipe_item)->verb);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_MIGRATE:
> red_printf("PIPE_ITEM_TYPE_MIGRATE");
> display_channel_marshall_migrate(display_channel, m);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_MIGRATE_DATA:
> display_channel_marshall_migrate_data(display_channel, m);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_IMAGE:
> red_marshall_image(display_channel, m, (ImageItem *)pipe_item);
> - release_image_item((ImageItem *)pipe_item);
> break;
> case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> display_channel_marshall_pixmap_sync(display_channel, m);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_PIXMAP_RESET:
> display_channel_marshall_reset_cache(display_channel, m);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_INVAL_PALLET_CACHE:
> red_reset_palette_cache(display_channel);
> red_marshall_verb((RedChannel *)display_channel, SPICE_MSG_DISPLAY_INVAL_ALL_PALETTES);
> - free(pipe_item);
> break;
> case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(pipe_item, SurfaceCreateItem,
> pipe_item);
> red_marshall_surface_create(display_channel, m, &surface_create->surface_create);
> - free(surface_create);
> break;
> }
> case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(pipe_item, SurfaceDestroyItem,
> pipe_item);
> red_marshall_surface_destroy(display_channel, m, surface_destroy->surface_destroy.surface_id);
> - free(surface_destroy);
> break;
> }
> default:
> red_error("invalid pipe item type");
> }
> +
> + display_channel_release_item_before_push(display_channel, pipe_item);
> +
> // a message is pending
> if (red_channel_send_message_pending(&display_channel->common.base)) {
> display_begin_send_message(display_channel, m);
> @@ -9082,21 +9070,20 @@ static void display_channel_hold_pipe_item(RedChannel *channel, PipeItem *item)
> }
> }
>
> -static void display_channel_release_item(RedChannel *channel, PipeItem *item, int item_pushed /* ignored */)
> +static void display_channel_release_item_after_push(DisplayChannel *display_channel, PipeItem *item)
> {
> - CommonChannel *common = SPICE_CONTAINEROF(channel, CommonChannel, base);
> + RedWorker *worker = display_channel->common.worker;
>
> - ASSERT(item);
> switch (item->type) {
> case PIPE_ITEM_TYPE_DRAW:
> case PIPE_ITEM_TYPE_STREAM_CREATE:
> - release_drawable(common->worker, SPICE_CONTAINEROF(item, Drawable, pipe_item));
> + release_drawable(worker, SPICE_CONTAINEROF(item, Drawable, pipe_item));
> break;
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> - red_display_release_stream_clip((DisplayChannel *)channel, (StreamClipItem *)item);
> + red_display_release_stream_clip(display_channel, (StreamClipItem *)item);
> break;
> case PIPE_ITEM_TYPE_UPGRADE:
> - release_upgrade_item(common->worker, (UpgradeItem *)item);
> + release_upgrade_item(worker, (UpgradeItem *)item);
> break;
> case PIPE_ITEM_TYPE_IMAGE:
> release_image_item((ImageItem *)item);
> @@ -9106,6 +9093,70 @@ static void display_channel_release_item(RedChannel *channel, PipeItem *item, in
> }
> }
>
> +static void display_channel_release_item_before_push(DisplayChannel *display_channel, PipeItem *item)
> +{
> + RedWorker *worker = display_channel->common.worker;
> +
> + switch (item->type) {
> + case PIPE_ITEM_TYPE_DRAW:
> + release_drawable(worker, SPICE_CONTAINEROF(item, Drawable, pipe_item));
> + break;
> + case PIPE_ITEM_TYPE_STREAM_CREATE: {
> + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> + red_display_release_stream(display_channel, agent);
> + break;
> + }
> + case PIPE_ITEM_TYPE_STREAM_CLIP:
> + red_display_release_stream_clip(display_channel, (StreamClipItem *)item);
> + break;
> + case PIPE_ITEM_TYPE_STREAM_DESTROY: {
> + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
> + red_display_release_stream(display_channel, agent);
> + break;
> + }
> + case PIPE_ITEM_TYPE_UPGRADE:
> + release_upgrade_item(worker, (UpgradeItem *)item);
> + break;
> + case PIPE_ITEM_TYPE_IMAGE:
> + release_image_item((ImageItem *)item);
> + break;
> + case PIPE_ITEM_TYPE_CREATE_SURFACE: {
> + SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem,
> + pipe_item);
> + free(surface_create);
> + break;
> + }
> + case PIPE_ITEM_TYPE_DESTROY_SURFACE: {
> + SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item, SurfaceDestroyItem,
> + pipe_item);
> + free(surface_destroy);
> + break;
> + }
> + case PIPE_ITEM_TYPE_INVAL_ONE:
> + case PIPE_ITEM_TYPE_VERB:
> + case PIPE_ITEM_TYPE_MIGRATE:
> + case PIPE_ITEM_TYPE_MIGRATE_DATA:
> + case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> + case PIPE_ITEM_TYPE_PIXMAP_RESET:
> + case PIPE_ITEM_TYPE_INVAL_PALLET_CACHE:
> + free(item);
> + break;
> + default:
> + PANIC("invalid item type");
> + }
> +}
> +
> +static void display_channel_release_item(RedChannel *channel, PipeItem *item, int item_pushed)
> +{
> + ASSERT(item);
> + if (item_pushed) {
> + display_channel_release_item_after_push((DisplayChannel *)channel, item);
> + } else {
> + red_printf("not pushed");
> + display_channel_release_item_before_push((DisplayChannel *)channel, item);
> + }
> +}
> +
> static void handle_new_display_channel(RedWorker *worker, RedsStream *stream, int migrate)
> {
> DisplayChannel *display_channel;
> --
> 1.7.4.4
>
More information about the Spice-devel
mailing list