Re: Signal emission from an object thread

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

On Fri, 2017-03-17 at 14:26 +0100, Emmanuel Pacaud wrote:
> Le ven. 17 mars 2017 à 9:52, Emmanuel Pacaud <emmanuel gnome org> a 
> écrit :
> > Le ven. 17 mars 2017 Ã  6:43, Tristan Van Berkom
> > <tristan vanberkom codethink co uk> a écrit :
> > > On Thu, 2017-03-16 at 10:42 +0100, Emmanuel Pacaud wrote:
> > > > Â I have an issue related to the use of g_signal_emit called
> > > > from an
> > > > Â object thread.
> > > I have used GWeakRef for references that threads make to objects 
> > owned
> > > by parent thread which may finalize with parent, to solve similar
> > > problems, but I dont believe I've tried using signals belonging
> > > to a
> > > thread spawning object from the thread itself.
> > >
> > > Another approach, if you want to keep using GSignal, would be to
> > > create
> > > a different object that is owned completely by the thread.
> > I think I will use this solution.
> I have just had a go at implementing something like that, but failed
> to 
> find the right way to do it. May be what I want to do is not
> possible:
>
> Currently, the 'new-buffer' signal is emitted by a ArvStream object, 
> which leads to the thread join issue I have described. What I would 
> like to do is to define a signal in ArvStream, but with a signal 
> callback that doesn't have ArvStream as the first parameter, but an 
> ArvBuffer. Do the signal callbacks always have the object emiting 
> instance in their parameters ?
No.

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 ?

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.

  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).

    The case of the boolean cancel is a bit exceptional, it can be
    set with an atomic operation. You may also consider using
    GCancellable for this.

  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.
    
    So, in your object initializer or constructed() method (which
    is where the bulk of your code that is currently in
    arv_uv_stream_new() should ideally be instead), you want to take
    note of the active thread context (the GMainContext).

    This makes sure that if some program decides to use your object
    in a mainloop which they created in a *different* thread, your
    code does not blindly assume it was created in the main thread
    but will still work.

    You just make the assertion that "The user which created this
    object did so in a thread with a GMainContext where they are
    running a GMainLoop".

    Once you've got this far, you'll want to share that parent thread
    context with your worker thread, so that ArvUvStreamThreadData
    now has a GMainContext pointer to the thread context of the parent.

    In order to deliver some notification to the calling object, you
    will now use:

        source = g_idle_source_new()
        g_source_set_callback(source,
                              parent_receive_func,
                              data,
                              free_data_func)
        g_source_attach(source, parent_thread_context)

    The object which created the thread, will then handle the
    'data' you sent in parent_receive_func the next time that
    that main thread is not busy and tries to go to sleep in
    it's GMainLoop.

  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.


Please note that GSignal by itself is _only_ a means for marshalling of
callbacks via it's implicit invocation mechanism, and asides from
protecting itself and being "thread aware", it does not care what
thread was active when somebody called g_signal_connect().

In other words, any handlers connected to a GSignal will be called
_right now_ when g_signal_emit() is called, in the active thread where
g_signal_emit() is called.

I hope this was helpful :)

Cheers,
    -Tristan