[Spice-devel] [PATCH] replace warning with comment in glz_usr_free_image

[Yonit Halperin]

On 09/19/2011 05:26 PM, Christophe Fergeau wrote:
> When running some xinerama tests, I got several
> glz_usr_free_image: error
> messages. Looking at the code, this error is reported when this
> function is called from a different DisplayChannelClient than the
> one which created the glz compressed image.
> When this happens, the backtrace is
>      at glz_encoder_dictionary.c:362
>      0x7fff940b6670) at glz_encoder_dictionary.c:449
>      image_type=LZ_IMAGE_TYPE_RGB32, image_width=512, image_height=256, image_stride=2048, first_lines=0x0,
>      num_first_lines=0, usr_image_context=0x7fff7420da40, image_head_dist=0x7fff9b2a3194)
>      at glz_encoder_dictionary.c:570
>      top_down=4, lines=0x0, num_lines=0, stride=2048, io_ptr=0x7fff740ea7c0 "  ZL", num_io_bytes=65536, usr_context=
>      0x7fff7420da40, o_enc_dict_context=0x7fff7420da60) at glz_encoder.c:255
>      drawable=0x7fff9b46bc08, o_comp_data=0x7fff9b2a3350) at red_worker.c:5753
>      0x7fff9b46bc08, can_lossy=0, o_comp_data=0x7fff9b2a3350) at red_worker.c:6211
>      0x7fff9b46bc08, can_lossy=0) at red_worker.c:6344
>      0x7fff74085c50, dpi=0x7fff7445b890, src_allowed_lossy=0) at red_worker.c:7046
>      0x7fff7445b890) at red_worker.c:7720
>      at red_worker.c:7964
>      at red_worker.c:8431
>
> Since the glz dictionary is shared between all the
> DisplayChannelClient instances, it can happen that the glz dictionary
> code decides to free an image from one thread while it was added from
> another thread (thread == DisplayChannelClient), so the error message
> that is printed is not an actual error. This commit removes this
> message and adds a comment explaining what's going on.
> ---
>   server/red_worker.c |    7 ++++++-
>   1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 3278824..c4bfe70 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -5482,7 +5482,12 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr, GlzUsrImageContext *im
>       if (this_cc == drawable_cc) {
>           red_display_free_glz_drawable_instance(drawable_cc, glz_drawable_instance);
>       } else {
> -        red_printf("error");
> +        /* The glz dictionary is shared between all DisplayChannelClient
> +         * instances, and glz_usr_free_image can be called by the
> +         * dictionary code  (glz_dictionary_window_remove_head). Thus this
> +         * function can be called from any DisplayChannelClient thread,
> +         * hence the need for this check.
> +         */
>           pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
>           ring_add_before(&glz_drawable_instance->free_link,
>                           &drawable_cc->glz_drawables_inst_to_free);

ACK, with small note. Currently The glz dictionary is shared between all 
DisplayChannelClient that belongs to the same client (i.e., for each 
client different dictionary, shared between all its display channels).
In the future we also want to share the dictionary between clients, in 
order to not recompress the same images for each client.

Yonit.