Re: [Discuss-gnuradio] Regarding lock protection when setting private variables in gnuradio blocks

[Marcus Müller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Marcus,

On 16.10.2014 16:54, address@hidden wrote:
> Is it not the case that a given instance of a block is only ever 
> executed by a single thread, so instance variables
> 
> are completely safe to modify in-flight?
> 
No; assume the following scenario:
block (instance) "estimator", in its work function, thinks it's cool
to update block (instance) "equalizer"'s internal state by calling
equalizer_sptr->set_taps(...). In the meantime, the equalizer's work
is running.

so, now equalizer_sptr->set_taps runs in estimator's thread, while it
modifies stuff that gets accessed from equalizer's thread.

Now, the solution to that is either using mutexes (which will, if not
done on a very high granularity, lead to blocks running effectively
single-threaded, because blocks block each other)
OR employing some notifier/listener scheme and let the blocks'
executors work through the updates after their work run in their own
thread. That way, you end up with a variant of the "new" style async
message passing framework, where messages cannot interfere with
running work functions, and message handling is guaranteed to be
reentrant-safe.

Greetings,
Marcus


> There are exceptions, of course, like in FFTW, only a single thread
> can be "planning" at a time, due to the
> 
> way FFTW does its thing.
> 
> On 2014-10-16 09:58, Tom Rondeau wrote:
> 
>> On Wed, Oct 15, 2014 at 2:55 PM, Achilleas Anastasopoulos
>> <address@hidden> wrote:
>> 
>>> Is "forecast()"" need to be protected in such a case as well?
>>> 
>>> just searching on the web i realized that no operation can be
>>> assumed atomic, so I guess every single set_ method (even if it
>>> assigns a float/int/short/char) needs to be protected...is this
>>> an overkill?
>>> 
>>> Achilleas
>> 
>> Achilleas,
>> 
>> So no about the forecast issue. Unless your forecast method uses
>> non-atomic variables that can be set using a function setter.
>> Marcus' response more completely outlines why forecast is thread
>> safe between itself and the work function.
>> 
>> On the atomic issue, yes, you're technically correct that there
>> really is no such thing as an atomic data type. C++11 more
>> officially defines concepts of atomic data types, and Boost
>> introduced it's atomic type (I believe) in 1.53
>> (http://www.boost.org/doc/libs/1_53_0/doc/html/atomic.html [1]).
>> However, for all intents and purposes, things like int, char,
>> float, double, etc behave fine in multithreaded environments. I'm
>> trying to remember where I went over this in some depth at one
>> point, but the result is that while they are not technically
>> atomic, they behave correctly -- and I wish I could explain more
>> thoroughly why right now.
>> 
>> We will be moving our Boost version requirements to >= 1.53 in
>> our 3.8 release, which means we can start using their atomic type
>> concept then.
>> 
>> Tom
>> 
>> On Wed, Oct 15, 2014 at 11:59 AM, Tom Rondeau <address@hidden>
>> wrote:
>> 
>> On Wed, Oct 15, 2014 at 11:49 AM, Achilleas Anastasopoulos
>> <address@hidden> wrote:
>> 
>> My question arose from a comment that Jonathan made on one of the
>> pull requests in gnuradio (#293).
>> 
>> If we have a set function in a gr block that sets some private
>> variable that is used in the work function, do we need to protect
>> it to make the whole operation thread safe?
>> 
>> Is this a standard practice in gnuradio blocks? eg, why is this
>> not used in "add_const_vXX_impl.h.t" ?
>> 
>> thanks Achilleas
>> 
>> If it's not atomic, then yes, you really should protect it. All
>> blocks have a mutex called d_setlock you can easily use for this:
>> 
>> 
>> gr::thread::scoped_lock lock(d_setlock);
>> 
>> Tom
> 
> _______________________________________________ Discuss-gnuradio
> mailing list address@hidden 
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio [2]
> 
> 
> 
> Links: ------ [1]
> http://www.boost.org/doc/libs/1_53_0/doc/html/atomic.html [2]
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio
> 
> 
> 
> _______________________________________________ Discuss-gnuradio
> mailing list address@hidden 
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJUP+TrAAoJEAFxB7BbsDrLqkUH/juNua+b3rID0Nt9/U75xX4K
uMsQNFK39OO3s3eACM/zFQQ6pL1g8sOVaHh76lwkfYP+bXS2sF+hnDmfLN9Zw5kG
VCbDWYuohMWBN0Ti0yMimocEDD9AO1IkaofC/p4/4HemlS6dNiJuviRWhbIki/6A
V9daWfs92nybtf610wy2k+QUqT+IpmKR+YdacE0wwHsmiBMwDU8+4lk76H/Wrclt
SG15zuvRHOcaBVfwY+Bkdnqub4e9uqc9JM8+Qo1+ayutkzQb5SeEWou1MbJ2QGdK
6SpsRGUSaffb/bMk06RZRJwazWZmboTRnIKC++FsIHmpvr72d/DGetOvlDfMe1I=
=fvJN
-----END PGP SIGNATURE-----