Re: [Discuss-gnuradio] fastnoise_source is not thread-safe

[Johannes Demel]

Hi Markus,

the whole gr::random class would be a candidate for a C++11 overhaul.
Though, C++11 is only supported on the next branch aka GR3.8. On GR3.7 
C++11 is not supported. To make matters worse, 3.7 requires to work with 
a fairly old Boost version. Somewhere in the 1.3x range if I recall 
correctly. Again, that makes it more difficult to write code here.

That fact that raw pointers are used in gr::random is due to its history.
I would suggest write a patch which uses unique pointers instead of raw 
pointers. Then issue a pull request and go through the discussion. I 
would expect this fix to be issued against maint. Jonathan is very 
supportive when it comes to PRs. He'll help you.

The same goes for fastnoise source. Write a small patch and create a PR.

Johannes

On 26.01.2018 00:45, Markus Wirsing wrote:
> Hello,
>
> I noticed that the fastnoise_source block is not thread safe, as it
> calls rand(), which keeps global state.
> This makes it non-reproducable between multiple runs once more than one
> such source is used.
>
> I also opened an issue about this on github:
> https://github.com/gnuradio/gnuradio/issues/1542
> However, there were no reactions to it.
>
> I'm considering to write a fix for it when I find the time.
> However, I'm not sure about the requirements that need to be met that it
> can later be included in gnuradio.
>
> Would this already require me to have a CRA?
> I'm also wondering whether C++11 is allowed in gnuradio.
> I guess it is not as I found this message on the mailing list:
> https://lists.gnu.org/archive/html/discuss-gnuradio/2014-06/msg00014.html
> It might be worth mentioning that on the coding guide in the wiki though:
> https://wiki.gnuradio.org/index.php/Coding_guide_impl
> It also might be worth considering to change that rule.
> C++11 is pretty well supported by now. And it does add some nice features.
>
> I think the easiest fix would be to use random::ran_int() in place of
> rand() or lrand48(). That way it would also be possible to get rid of
> the (slightly) biased method of using a linear congruence to reduce the
> range.
>
> However, while looking at the code in random.cc
> https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/math/random.cc
> I noticed that it keeps raw pointers to the different random generators.
> I don't see why this is done. If I don't misunderstand anything, it
> would work equally well to have the objects as members in the class.
> With the added benefit of having them correctly deconstructed in the
> case of exceptions. Even though that may not be of much relevance in
> this case.
> But it at least would make sense to use unique pointers instead of raw
> pointers.
> So if I find the time to create a fix for the other problem, would some
> cleanup in this place be appreciated too?
>
> I have no experience so far with submitting code to open source projects.
> These issues seem to be a good starting point. However, before investing
> time, I want to make sure my work can actually be used.
>
> Appreciating any feedback on the proposed changes and on the necessary
> formalities,
> Markus
>
> _______________________________________________
> Discuss-gnuradio mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio