[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Johannes Demel
Subject: Re: [Discuss-gnuradio] fastnoise_source is not thread-safe
Date: Fri, 26 Jan 2018 09:43:57 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0

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.


On 26.01.2018 00:45, Markus Wirsing wrote:

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:
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:
It might be worth mentioning that on the coding guide in the wiki though:
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

However, while looking at the code in 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
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

Discuss-gnuradio mailing list

reply via email to

[Prev in Thread] Current Thread [Next in Thread]