Re: Signal emission from an object thread

[Emmanuel Pacaud <emmanuel gnome org>]

Le sam. 18 mars 2017 à 6:29, Tristan Van Berkom 
<tristan vanberkom codethink co uk> a écrit :
> I took a brief look into your code, your thread it calling:
>
>     arv_stream_push_output_buffer()
>
> Which is emitting a gsignal. I'm not going to get a full handle on 
> what
> your code is trying to do, but I can only presume that you intend for
> that to let your parent thread know that a buffer is ready ?

Thanks a lot to have a look at my code.

> Ok, well, here are a few tips I think you can use to correct your 
> code.
>
>   o The `gboolean cancel` is wrong, setting this is not a guaranteed
>     atomic operation.
>
>     There are some facilities for atomic integers which you can use,
>     such as g_atomic_int_set/get(), which act as a memory barrier 
> which
>     is less overhead than using mutexes.

I understand the setting of cancel is not atomic, but I fail to see 
what can possibly go wrong in this particular case. There is always 
only one thread reading this value, and while this thread is alive, 
only one event can happen, when the value goes from FALSE to TRUE.
I will replace it with an atomic operation, but still, I'd love to have 
an example where the use of a simple gboolean can lead to a wrong 
beheviour.

>   o Whenever sharing data with your thread, always protect it with
>     g_mutex_lock() / g_mutex_unlock(), (there are also reader/writer
>     locks available but I dont see you needing this here).

If I have done this correctly, there shouldn't be any data sharing, 
except for the cancel boolean, when the thread is alive. I will double 
check.

>   o If you want to communicate something back to the thread in which
>     your main object was created, from the child worker thread, use
>     the GMainLoop for this, that is easiest.

Thanks for the suggestion and the detailed explanation. After more 
thinking, I'm not completely why I have implemented this signal. It was 
for the multicast nature of signals, but in real life there souldn't be 
any interresting use case of this property.

>   o There is something relatively new called GTask which has appeared,
>     and it might take care of some of the above for you so that there
>     are less lines of code overall, I have not really tried working
>     with GTask since it's creation.

I know this nice API and already used it in another project. But I can 
not use it here as I'd like to retain the glib requirement to 2.26, 
while GTask appeared in glib 2.36.

> I hope this was helpful :)

Sure !

Thanks again.

        Cheers,

                Emmanuel.