On Wed, Jul 25, 2012 at 04:50:15PM +0300, Yonit Halperin wrote:
> After marshalling MSG_STREAM_CREATE, there is no need to ref and
> unref stream->current before and after completing the sending of the
> message (correspondingly). The referencing is unnecessary because all
> the data that is required from the drawable (the clipping), is copied
> during marshalling, and no field in the drawable is referenced (see
> spice_marshall_msg_display_stream_create).
>
> Moreover, the referencing was bugous:
> While display_channel_hold_pipe_item and
> display_channel_client_release_item_after_push referenced and
> dereferenced, correspondingly, stream->current, stream->current might
> have changed in between these calls, and then we ended up with one drawable
> leaking, and one drawable released before its time has come (which
> of course led to critical errors).
Good one, ACK.
> ---
> server/red_worker.c | 16 +---------------
> 1 files changed, 1 insertions(+), 15 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 9009462..9330fff 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -8552,7 +8552,7 @@ static void red_display_marshall_stream_start(RedChannelClient *rcc,
>
> agent->last_send_time = 0;
> spice_assert(stream);
> - red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CREATE, &agent->create_item);
> + red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_CREATE, NULL);
> SpiceMsgDisplayStreamCreate stream_create;
> SpiceClipRects clip_rects;
>
> @@ -9955,13 +9955,6 @@ static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item
> case PIPE_ITEM_TYPE_DRAW:
> ref_drawable_pipe_item(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
> break;
> - case PIPE_ITEM_TYPE_STREAM_CREATE: {
> - StreamAgent *stream_agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> - if (stream_agent->stream->current) {
> - stream_agent->stream->current->refs++;
> - }
> - break;
> - }
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> ((StreamClipItem *)item)->refs++;
> break;
> @@ -9985,13 +9978,6 @@ static void display_channel_client_release_item_after_push(DisplayChannelClient
> case PIPE_ITEM_TYPE_DRAW:
> put_drawable_pipe_item(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
> break;
> - case PIPE_ITEM_TYPE_STREAM_CREATE: {
> - StreamAgent *stream_agent = SPICE_CONTAINEROF(item, StreamAgent, create_item);
> - if (stream_agent->stream->current) {
> - release_drawable(worker, stream_agent->stream->current);
> - }
> - break;
> - }
> case PIPE_ITEM_TYPE_STREAM_CLIP:
> red_display_release_stream_clip(worker, (StreamClipItem *)item);
> break;
> --
> 1.7.7.6
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel