Re: Signal emission from an object thread

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

On Sat, 2017-03-18 at 07:25 +0100, Emmanuel Pacaud wrote:
> 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.

There are a few reasons to take care of how you access memory shared
between CPU cores, atomic integers basically achieve two things:

  a.) Forces the compiler to behave how you want.

      By not reordering memory reads and writes around the access of
      a specific variable, which may be reordered for optimization.

      Also never make any assumption that the variable will not change
      if not assigned in code.

      This part is mostly taken care of by referring to that variable
      as 'volatile' (which the atomic ops will do for you).

  b.) Enforce memory barriers 

      I'm a bit rusty here, but this basically issues some instructions
      which will ensure synchronization of multiple devices accessing
      main memory (ram). In this case, especially if your two threads
      are running on separate cores, you want to be sure that local CPU
      caches are synchronized properly so that the other core will
      notice the change as soon as possible.

      At write time:

         /* Ensure this is visible to other hardware after write */
         modify memory
         barrier()

      At read time:

         /* Ensure local hardware caches are in sync before read */
         barrier()
         read memory

It's quite possible that your code seems to work fine without this, but
it will certainly behave differently on different machine
architectures. It's also quite possible that as it stands, your child
thread is not reading the changed 'cancel' variable as soon as it could
be (it may loop more than it needs to, until the cancel variable is
really updated).

Cheers,
    -Tristan