Arduino's random() might not be reentrant or thread safe, but does it matter?

Projects Programming
1

I've got some code which calls random() both in the main loop and in an ISR. I just realised this could lead to a re-entrancy problem.

So I looked up the arduino source code for random(with_some, possible_arguments). It is found inside /hardware/arduino/avr/cores/arduino/WMath.cpp.

This in turn calls random() which is part of the avr-libc.

So I looked that up, and it turns out that avr-libc's random() simply runs:
do_random(&next)
where next is a static variable. By the time do_random(&next) has finished the value in that static unsigned long has been changed.

To my understanding this means "trouble" could happen if an ISR is triggered during a time when the program's main loop is calling random(), and if the ISR then calls random() for itself.

But given the whole purpose of the random() function is to generate random numbers is this "trouble" ok?

It doesn't look like any pointers can become corrupted such that they could end up over-writing memory tht belongs to other variables, it just seems that the value of the unsigned long called next can be corrupted. And a corrupt value for a random variable is still a random variable, infact more random now.

With a function doing a calculation you wanted predictable resuts from this would be a problem, but for generating randomess is there any risk of something which could actually mess with a program's operation?

So this would destroy any hope for predictable randomness, like if you wanted to run repeatable monte-carlo modelling, but if you just want some randomness (doesn't need to be cryptographic grade, just random enough to give a good chance that separate arduino's running the same code will not start doing things at exactly the same time after powering up) is this lack of thread-safety and re-entrancy actually a good thing?
Thanks

2

It seems pointless to call random() in an ISR. Unless you have a very unusual requirement for doing so, just set a global flag and let the main loop handle it.

3

A relatively unusual requirement, a situation where the main loop does things slowly enough that if the random were done in the main loop it might not be ready for when it was needed. The ISR is likely to happen many times for each single loop of the main loop. As it stands it is done just before leaving the ISR, so a new random variable is ready for use in the ISR next time the ISR is triggered. But as I discuss above, given what the purpose of random() is, is the lack of thread-safety and re-entrancy a problem? it sounds more like a possibilty for making the randomness a little more random? The randomness needed in the main loop is for entirely separate purposes than that in the ISR, whether they sometimes briefly hold the same value isn't a problem though.
Thanks

4

Put a noInterrupts() / interrupts() pair around your use of random() in the non-interrupt code. Then you won't have to worry about it.

5

Yes, that would work. I think a call to random() is already quicker than the atomic blocks I've got in use elsewhere.

But I would still like to understand whether using random like this is problem-free, simply because inconsistent numerical results are ok from a random numebr generator function.

It could be hard to test random() in an ISR colliding with random() in a main loop in the real world, as one wouldn't really know what effect to look for to know if it was causing problems. And if it were causing over-writing of other variables (impossible here ? or not?) that would be particularly hard to identify.

6

Not if it skews the distribution of the "random" numbers from what you get if the function is called properly.

So, why bother? Just write your code so that it doesn't happen.

7

No. Of course it's not okay. It's not even worth the time to analyze the possible outcomes given the price of avoiding the problem is about three machine instructions.

12

If you dont want to calculate random in your interrupt and you need randoms more than your main loop does you could create a random buffer array.

This gets filled in your main loop. The interrupt then grabs the next needed values from the buffer and your loop fills up the buffer whenever its not full.

13

Thanks everyone for a long and detailed discussion, but a call to random() takes about 50us, which is longer than I want to have an atomic block in the main code last for. I'm in a somewhat weird situation where an interrupt must react very fast to an event, but then it is ok in my application for the interrupt to take a while during the later things it does befoe it returns. Hence my calls to random() within the interrupt.

Now, my application doesn't need cryptographic grade randomness, and it doesn't need repeatable randomness. It does need 8 bits of randomness though.

So, can anyone take a close look at the avr-libc function underneath this all and confirm my suspicio as in my irst post of this thread. Yes that function uses pointers, but I don't think it changes the addresses at all? just the values at those addresses? So if random during an interrupt overlaps with random called in the main code, and they both try to do their things... is there any chance of really nasty undefiend behaviour? (Anything where it over-writes another variable, or crashes running code, or steals a massive amount of SRAM...)
Or is any undefiend behaviour in such circumstances confined to causing undefined and chaotic values for the random variable, and doing nothing else?

In my application I'm ok with "undefined behaviour" in my randomness, as long as the undefiend is confined to the value which the random variable gets given and doesn't have side effects on other parts of the code. Even if the randomness became utterly non-random briefly in such circumstances I'd probably be ok again within the next few times that random() was called, and the interrupt colliding precisely with random during the main loop is unlikely anyway. That's why making sure there are no side-effects, over-writing of other variables or memory corruption matters to me, but if the randomness itself becomes "more random" or "less random" or doesn't properly match the distribution any more for a few subsequent random() calls, then for this application: "so what".

That's what I'm trying to be sure of here, no side effects which would affect things beyond simply the "random" result given and the value of the random "seed" variable kept in the background.

Thanks

14

Neither of those is true.

I struggling to understand why you don't just lift do_random then call it with different storage; one next for the ISR and one for the non-interrupt code. Obviously, you will have to use two different seeds.

Are you disabling the millis / micros timer?

Given your desire for the best possible performance and the need for a mere 8 bits of randomness I'm struggling to understand why you are not just using a 256 element array preloaded with shuffled values.

15

I have no idea what your interrupt is doing but if the majority of your program is running in your interrupt youve got something completely wrong and is going against the purpose of interrupts.

What if other interrupts start happening while your still in your interrupt. You can get to a point of your main loop not even running anymore

You wont end up with a stable project. There will be a way to restructure your project to fix this.

16

So you say that it's okay to call the "too much time consuming" random() function very often in an ISR until the result is used once in loop()?

So you use random() both in your main code, without complaining about it using too much time, and for what purpose more often in an ISR?

Have you tested my suggestion of using the low 8 bits of micros() instead of random()?

17

As already mentioned, there's likely something fundamentally wrong with the architecture of your code.

18

I can imagine a long running loop(), e.g. lengthy update of a display, and a short event that requires recognition by interrupt.

But even that is no excuse for banning random() from loop().

19

Yes, but that's the exact opposite of the architecture that @Infraviolet described.

20

Best answer I've heard yet. Problem solved. OP needs to move on.

21

Ok, the source code of the function used inside random() is here:

As far as I can tell the pointer stays constant, all that ever gets changed is the value at the place in memory which that pointer points to.

But I'm a little mystified by the line:
return ((*ctx = x) % ((unsigned long)RANDOM_MAX + 1));

Does that mean it checks whether the value at the place ctx points to can be set to 1? Then does a modulo division of the 1 or 0 result by RANDOM_MAX+1? It looks very like one of those accidents one makes where one puts x=y in an if loop and it always returns true. Does this disguise something where the address being pointed to is indeed changing?

So, I ask again, what is the WORST THAT COULD HAPPEN with use of random() in both the main loop and in an ISR. If it is messing up of the value of the variable which ctx points to, I don't care, yes it won't be ideal randomness, but the chances of an ISR calling random() right while the main loop is doing it too are pretty low. If it is messing up of the carried over random seed value, or the value returned by random() a few times, again I'm fine. But if it is corruption of a pointer such that memory used by variables unrelated to random could end up getting over-written, or if it is something which would steal a bunch of SRAM each time this happened and never release it again, then I'm concerned. Is the worst case scenario for THIS SPECIFIC CASE of violating re-entrancy/thread-safety anything worse than simply a "random" variable's value becoming a different value than it ought to be under the strict interpretation of what a PRNG ought to do?

Thanks

P.S. the 50us figure: I put a port manipulation instruction (one to go high, the other to go low) either side of a call to random() and I used memory barrier ( #define barrier() asm volatile("" ::: "memory") ) instructions to ensure this part of code happened exactly in order. On an oscilloscope I timed 50 us from pin rise to pin fall.

22

It returns x modulo (RANDOM_MAX + 1), i.e. something between 0 and10 for RANDOM_MAX = 10.

From the source code it becomes clear why an AVR controller spends so much time, with 3 emulations of / and %. Try to find an less demanding algorithm.

23

Why do you care given that a much better solution has already been provided?

24

DrDiettrich: thanks, It returns x % (RANDOM_MAX + 1), but it does something to the ctx pointer at the same time?

gfvalvo: I've rather caught the "urge to know" on this matter. Also, I'm working on something for later use on at ATTiny, there is neither 256B of free SRAM to hand once all other functionalities are considered, nor 256B of free flash left (once all other code features compiled) to put such a table in progmem. Things work, and work well it appears, but I'm concerned about the remote possibility of random() in the ISR being called right whilst the main loop is doing random(), if it corrupts the random value and/or seed on occasion that's not a problem, so long as it doesn't over-write other variables or act as a form of memory leak, worsening on (still unlikely) repeated occurences until there's no SRAM left.