> On 9 Feb 2016, at 17:14 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
>
> On Thu, 2015-10-29 at 17:26 +0200, Dmitry Fleytman wrote:
>> From: Kirill Moizik <kmoizik@xxxxxxxxxx>
>>
>> This commit introduces channel mutex to allow usage of
>> channel objects in mutithreaded environments.
>>
>> This mutex will be used to protect non thread safe
>> usbredir functions and data structures.
>>
>> Signed-off-by: Kirill Moizik <kmoizik@xxxxxxxxxx>
>> Signed-off-by: Dmitry Fleytman <dfleytma@xxxxxxxxxx>
>> ---
>> src/channel-usbredir-priv.h | 4 ++++
>> src/channel-usbredir.c | 19 +++++++++++++++++++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/src/channel-usbredir-priv.h b/src/channel-usbredir-priv.h
>> index 2c4c6f7..c987474 100644
>> --- a/src/channel-usbredir-priv.h
>> +++ b/src/channel-usbredir-priv.h
>> @@ -51,6 +51,10 @@ void
>> spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel);
>>
>> libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel
>> *channel);
>>
>> +void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel);
>> +
>> +void spice_usbredir_channel_unlock(SpiceUsbredirChannel *channel);
>> +
>> void spice_usbredir_channel_get_guest_filter(
>> SpiceUsbredirChannel *channel,
>> const struct usbredirfilter_rule **rules_ret,
>> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
>> index c236ee7..8cf3387 100644
>> --- a/src/channel-usbredir.c
>> +++ b/src/channel-usbredir.c
>> @@ -79,6 +79,7 @@ struct _SpiceUsbredirChannelPrivate {
>> GSimpleAsyncResult *result;
>> SpiceUsbAclHelper *acl_helper;
>> #endif
>> + GMutex *flows_mutex;
>> };
>>
>> static void channel_set_handlers(SpiceChannelClass *klass);
>> @@ -107,6 +108,8 @@ static void
>> spice_usbredir_channel_init(SpiceUsbredirChannel *channel)
>> {
>> #ifdef USE_USBREDIR
>> channel->priv = SPICE_USBREDIR_CHANNEL_GET_PRIVATE(channel);
>> + channel->priv->flows_mutex = g_new0(GMutex, 1);
>> + g_mutex_init(channel->priv->flows_mutex);
>> #endif
>> }
>>
>> @@ -182,6 +185,10 @@ static void spice_usbredir_channel_finalize(GObject *obj)
>>
>> if (channel->priv->host)
>> usbredirhost_close(channel->priv->host);
>> +#ifdef USE_USBREDIR
>> + g_mutex_clear(channel->priv->flows_mutex);
>> + g_free(channel->priv->flows_mutex);
>> +#endif
>>
>> /* Chain up to the parent class */
>> if (G_OBJECT_CLASS(spice_usbredir_channel_parent_class)->finalize)
>> @@ -561,6 +568,18 @@ static void *usbredir_alloc_lock(void) {
>> #endif
>> }
>>
>> +void spice_usbredir_channel_lock(SpiceUsbredirChannel *channel)
>> +{
>> + g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
>> + g_mutex_lock(channel->priv->flows_mutex);
>> +}
>> +
>> +void spice_usbredir_channel_unlock(SpiceUsbredirChannel *channel)
>> +{
>> + g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
>> + g_mutex_unlock(channel->priv->flows_mutex);
>> +}
>> +
>> static void usbredir_lock_lock(void *user_data) {
>> GMutex *mutex = user_data;
>>
>
>
>
> The code itself looks fine, but again, it's hard to judge the design or
> necessity of the mutex without seeing how it's used. I'll come back to this
> after I've reviewed the rest of the patches. Also, why did you choose the name
> "flows_mutex"? The term "flow" is not used elsewhere in this file as far as I
> can tell.
Indeed the name may be better. Renaming device_connect_mutex.
>
> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel