[Spice-devel] [PATCH v7 05/14] usbredir: Protect data accessed by asynchronous redirection flows

[Jonathon Jongsma]

Previously acked

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Tue, 2016-03-08 at 16:05 +0200, Dmitry Fleytman wrote:
> From: Kirill Moizik <kmoizik at redhat.com>
> 
> This commit adds locking to ensure thread safety required
> after start/stop redirection flows moved to separate threads.
> This is done in preparation to following commits that
> will introduce actual multithreaded access to corresponding
> routines.
> 
> Signed-off-by: Kirill Moizik <kmoizik at redhat.com>
> Signed-off-by: Dmitry Fleytman <dfleytma at redhat.com>
> ---
>  src/channel-usbredir.c   |  9 ++++++++-
>  src/usb-device-manager.c | 12 ++++++++++--
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index daa35ec..9491f60 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -656,13 +656,18 @@ static void usbredir_handle_msg(SpiceChannel *c,
> SpiceMsgIn *in)
>      priv->read_buf = buf;
>      priv->read_buf_size = size;
>  
> +    spice_usbredir_channel_lock(channel);
> +
>      r = usbredirhost_read_guest_data(priv->host);
>      if (r != 0) {
>          SpiceUsbDevice *spice_device = priv->spice_device;
>          gchar *desc;
>          GError *err;
>  
> -        g_return_if_fail(spice_device != NULL);
> +        if (spice_device == NULL) {
> +            spice_usbredir_channel_unlock(channel);
> +            return;
> +        }
>  
>          desc = spice_usb_device_get_description(spice_device, NULL);
>          switch (r) {
> @@ -699,6 +704,8 @@ static void usbredir_handle_msg(SpiceChannel *c,
> SpiceMsgIn *in)
>  
>          g_error_free(err);
>      }
> +
> +    spice_usbredir_channel_unlock(channel);
>  }
>  
>  #endif /* USE_USBREDIR */
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index f4e48eb..4d376b6 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -1326,9 +1326,13 @@ static SpiceUsbredirChannel
> *spice_usb_device_manager_get_channel_for_dev(
>  
>      for (i = 0; i < priv->channels->len; i++) {
>          SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
> +        spice_usbredir_channel_lock(channel);
>          libusb_device *libdev = spice_usbredir_channel_get_device(channel);
> -        if (spice_usb_manager_device_equal_libdev(manager, device, libdev))
> +        if (spice_usb_manager_device_equal_libdev(manager, device, libdev)) {
> +            spice_usbredir_channel_unlock(channel);
>              return channel;
> +        }
> +        spice_usbredir_channel_unlock(channel);
>      }
>  #endif
>      return NULL;
> @@ -1730,9 +1734,13 @@
> spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>      /* Check if there are free channels */
>      for (i = 0; i < priv->channels->len; i++) {
>          SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
> +        spice_usbredir_channel_lock(channel);
>  
> -        if (!spice_usbredir_channel_get_device(channel))
> +        if (!spice_usbredir_channel_get_device(channel)){
> +            spice_usbredir_channel_unlock(channel);
>              break;
> +        }
> +        spice_usbredir_channel_unlock(channel);
>      }
>      if (i == priv->channels->len) {
>          g_set_error_literal(err, SPICE_CLIENT_ERROR,
> SPICE_CLIENT_ERROR_FAILED,