[Spice-devel] [spice-devel][spice PATCH V2]Lock the pixmap image cache for the entire fill_bits call

[Christophe Fergeau]

On Fri, Jun 26, 2015 at 11:59:13AM -0400, Sandy Stutsman wrote:
> Locking the individual calls that access the pixmap cache in fill_bits is
> not adequately thread safe.  Often a windows guest with multiple monitors
> will be sending the same image via different threads.  Both threads can
> be in fill_bits at the same time making changes to the cache for the same
> image.  This can result in images being deleted before all the client
> channels are finished with them or with the same image being send multiple
> times.  Here's what can happen with out the lock in fill_bits
> 
> On the server in red_worker.c:fill_bits
>  Thread 1 calls pixmap_cache_hit for Image A and finds it isn't in cache
>  Thread 2 calls pixmap_cache_hit for Image A and finds it isn't in cache
> 
>  Thread 1 adds Image 1 to pixmap_cache (1x)
>  Thread 2 adds Image 1 to pixmap_cache (2x)
> 
> On the client
>  Channel 1 adds Image A to image_cache (1x)
>  Channel 2 replaces Image A in image_cache (1x)
> 
> On server
>  Thread 1 sends Image A rendering commands
>  Thread N removes Image A from pixmap_cache (image remains - 1x)
>  Thread 2 sends Image A rendering commands
> 
> On client
>  Channe1 renders from Image A
>  Channel N removes Image a from image_cache (image is completely removed)
>  Channel2 render command hangs waiting for Image A
> ---
> Changes since V1
> - Incorporated Christophe's changes to make naming consistent
> - Fleshed out commit message

ACK, though I'll squash a rename from set_lossy to unlocked_set_lossy()
in this before pushing as this commit removes the locking from
_set_lossy.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150629/8d52753e/attachment.sig>