Is C# Random Number Generator thread safe?

Question

Is C#'s Random.Next() method thread safe?

Solution 1

There's nothing special done in the Next method to achieve thread safety. However, it's an instance method. If you don't share instances of Random across different threads, you don't have to worry about state corruption within an instance. Do not use a single instance of Random across different threads without holding an exclusive lock of some sort.

Jon Skeet has a couple nice posts on this subject:

StaticRandom
Revisiting randomness

As noted by some commentators, there is another potential problem in using different instances of Random that are thread-exclusive, but are seeded identically, and therefore induce the identical sequences of pseudorandom numbers, because they may be created at the same time or within close temporal proximity of each other. One way to alleviate that issue is to use a master Random instance (which is locked by a single thread) to generate some random seeds and initialize new Random instances for every other thread to use.

Solution 2

No, using the same instance from multiple threads can cause it to break and return all 0's. However, creating a thread-safe version (without needing nasty locks on every call to Next()) is simple. Adapted from the idea in this article:

public class ThreadSafeRandom
{
    private static readonly Random _global = new Random();
    [ThreadStatic] private static Random _local;

    public int Next()
    {
        if (_local == null)
        {
            int seed;
            lock (_global)
            {
                seed = _global.Next();
            }
            _local = new Random(seed);
        }

        return _local.Next();
    }
}

The idea is to keep a separate static Random variable for each thread. Doing that in the obvious way fails, however, because of another issue with Random - if multiple instances are created at nearly the same time (within about 15ms), they will all return the same values! To fix this, we create a globally-static Random instance to generate the seeds used by each thread.

The above article, by the way, has code demonstrating both of these issues with Random.

Solution 3

The offical answer from Microsoft is a very strong no. From http://msdn.microsoft.com/en-us/library/system.random.aspx#8:

Random objects are not thread safe. If your app calls Random methods from multiple threads, you must use a synchronization object to ensure that only one thread can access the random number generator at a time. If you don't ensure that the Random object is accessed in a thread-safe way, calls to methods that return random numbers return 0.

As described in the docs, there is a very nasty side effect that can happen when the same Random object is used by multiple threads: it just stops working.

(i.e. there is a race condition which when triggered, the return value from the 'random.Next....' methods will be 0 for all subsequent calls.)

Solution 4

No, it's not thread safe. If you need to use the same instance from different threads, you have to synchronise the usage.

I can't really see any reason why you would need that, though. It would be more efficient for each thread to have their own instance of the Random class.

Solution 5

Another thread safe way is to use ThreadLocal<T> as follows:

new ThreadLocal<Random>(() => new Random(GenerateSeed()));

The GenerateSeed() method will need to return a unique value each time it is called to assure that the random number sequences are unique in each thread.

static int SeedCount = 0;
static int GenerateSeed() { 
    return (int) ((DateTime.Now.Ticks << 4) + 
                   (Interlocked.Increment(ref SeedCount))); 
}

Will work for small numbers of threads.

View more solutions

Comments

[Rangoric]

Yeah the RNGCryptoServiceProvider is gold. Much better than Random, though there is a bit more legwork to make it spit out a particular type of number (since it generates random bytes).

[Guffa]

@Rangoric: No, it's not better than Random for any purpose where either can be used. If you need randomness for encryption purposes the Random class is not an option, for any purpose where you can choose, the Random class is faster and easier to use.

[Rangoric]

@Guffa ease of use is a one time thing though as I already stuffed it into a library, so is not really a good point. Being faster is a valid point, though I'd rather have the real randomness as opposed to looks good randomness. And for that I also get to have it be thread safe. Although now I intend to test this to see how much slower it is (Random produces doubles and converts them to what you ask for so it may even depend on exactly what number range you need)

[Rangoric]

In C# I am finding that with a very basic RNGCrypto implementation it's about 100:1 depending on the exact number being looked for (127 does twice as good as 128 for instance). Next I plan to add threading and see how it does (why not :) )

[Rangoric]

Update to above statement. I got it to a 2:1 for ranges of under 256 values, and does better the closer to a factor of 256 the number we want is.

[gap]

But in this case, doesn't it make sure that the method doesn't need to be thread-safe.... each thread will access their own copy of the object.

[Admin]

Documentation that you quoted is actually incorrect. I believe it's a machine-generated content. The Community Content to that topic on MSDN contains a lot of info why the Random type is not thread safe and how to solve this problem (either with cryptography or by using "semaphores")

[JSWork]

This can bite you in the ass if you have a unit test object and want to generate tons of test objects at once. The reason for this is that many people make Random a global object for ease of use. I just did that and rand.Next() kept generating 0 as a a value.

[Guffa]

@JSWork: I don't really follow what you mean. What do you refer to when you say "this"? If I understand your last sentence correctly, you accessed the object across threads without synchronising it, which would explain the result.

[JSWork]

You are correct. I apologize - I phrased this poorly. Not doing what you mentioned can bite you in the ass. As cautionary note, readers should be careful when making a new random object for every thread as well - random objects use the current time as their seed. There is a 10ms gap between seed changes.

[Guffa]

@JSWork: Yes, there is a timing issue if the threads are started at the same time. You can use one Random object in the main thread to provide seeds for creating Random objects for the threads to get around that.

[Edward Brey]

++SeedCount introduces a race condition. Use Interlocked.Increment instead.

[BlueRaja - Danny Pflughoeft]

"If you don't share instances of Random across different threads, you don't have much to worry about." - This is false. Because of the way Random was created, if two separate instances of Random are created on two separate threads at nearly the same time, they will have the same seeds (and thus return the same values). See my answer for a workaround.

[mmx]

@BlueRaja I was specifically targeting the state corruption issue within a single instance. Of course, as you mention, the orthogonal problem of statistical relation across two distinct Random instances requires further care.

[apokryfos]

If you are to make a separate random for each thread use something more unique to each thread for a seed, such as thread id or time+thread id or similar.

[weston]

Nice, but don't like the way you need to create a ThreadSafeRandom to use this. Why not use a static property with a lazy getter which contains the construtors code at present. Idea from here: confluence.jetbrains.com/display/ReSharper/… Then the whole class can be static.

[BlueRaja - Danny Pflughoeft]

@weston: That is usually considered an anti-pattern. Aside from losing all OOP benefits, the primary reason is that static objects cannot be mocked, which is vital for unit-testing any classes that use this.

[weston]

Does that then mean you're against all static classes and static public methods?

[weston]

You can always add a layer of indirection when needed, e.g. add an IRandom, and a class that redirects calls to a static at runtime, this would allow mocking. Just for me, it all the members are static, that implies I should have a static class, it tells the user more, it says that each instance is not separate sequence of random numbers, it is shared. This is the approach I have taken before when I need to mock a static framework class.

[BlueRaja - Danny Pflughoeft]

@weston: Static classes are basically singletons, but worse in almost every way (singletons are also pretty rarely used nowadays - DI is preferred). The only time you should really ever have a static class is when you have a group of functions that you want logically grouped together, but which share no state (like the Math class) - this is not the case here. This is the general consensus among programmers, not just my opinion.

[Terry]

So if I use this implementation of a Random, and declared and used it in a website, would every instance of the website use the same Random generator and thus generate random numbers across all instances to the site?

[Terry]

Thanks. Fine to add overload like this? public int Next( int minValue, int maxValue ) { return _local.Next( minValue, maxValue ); }

[Chris Marisic]

What is the purpose of PositionLock = new Lazy<object>(() => new object());? Shouldn't this just be SyncRoot = new object();?

[Chris Marisic]

As noted by OP, this will work with a limited number of threads, this would likely be a poor choice inside of ASP.NET

[Jodrell]

@ChrisMarisic, I was following the pattern linked below. However, there is minimal benefit, if any, to the lazy instantiation of the lock so your suggestion seems reasonable. csharpindepth.com/articles/general/singleton.aspx#lazy

[Laie]

Code won't work if you actually use it from multiple threads since _local is not created everytime it is accessed: NullRefereceException.

[Mick]

I have no idea why this is marked as the answer! Q: "Is Random.Next thread safe?" A: "If you only use it from one thread, then yes it is thread safe".... Worst answer ever!

[Mick]

This should be the answer

[AaA]

There is a nastier side effect of using different instances of random object. it returns the same generated number for multiple threads. from same article: Instead of instantiating individual Random objects, we recommend that you create a single Random instance to generate all the random numbers needed by your app. However, Random objects are not thread safe.

[Mark Amery]

-1; GUIDs generated with NewGuid are practically guaranteed to be unique, but the first 4 bytes of those GUIDs (which is all that BitConverter.ToInt32 looks at) are not. As a general principle, treating substrings of GUIDs as unique is a terrible idea.

[Mark Amery]

The only thing that redeems this approach in this particular case is that .NET's Guid.NewGuid, at least on Windows, uses version 4 GUIDs, which are mostly randomly generated. In particular, the first 32 bits are randomly generated, so you're essentially just seeding your Random instance with a (presumably cryptographically?) random number, with a 1 in 2 billion collision chance. It took me hours of research to determine that, though, and I still have no idea how .NET Core's NewGuid() behaves on non-Windows OSes.

[Mark Amery]

@MTG Your comment is confused. Random has no static members, so the quoted docs are effectively stating that all of its members are "not guaranteed to be thread safe". You state that this is incorrect, then back this up by saying that... Random is not thread safe? That's pretty much the same thing the docs said!

[Mark Amery]

The (unavoidable) cast to int kind of defeats the point of this. Java's Random is seeded with a long, but C#'s just takes an int... and worse still, it uses the absolute value of that signed int as the seed, meaning there's effectively only 2^31 distinct seeds. Having good long seed generation is kind of a waste if you're then throwing away most bits of the long; randomly seeding C#'s random, even if your random seed is perfectly random, still leaves you with a roughly 1 in 2 billion chance of collision.

[Mark Amery]

I'd be cautious about using this approach. There are only around 2 billion possible seeds for Random (precisely, there are 2^31 + 1, since the Random constructor takes a signed int and then throws the sign away before using it); for some applications, even if cryptographically-secure randomness isn't needed, a 1 in 2 billion chance that two threads will generate identical "random" sequences may be undesirable. Incrementing a statically-stored previous seed seems like a better general practice to me than randomly generating seeds as you do here.

[Glenn Slayden]

@MarkAmery The OP did not specify whether cryptographic quality was required, so I feel that the answer is still useful as a one-liner for quick coding in non-critical situations. Based on your first comment, I modified the code to avoid the first four bytes.

[Seattle Leonard]

Bangs head on desk. Yes you are correct "That's pretty much the same thing the docs said!"

[Mark Amery]

Using the second 4 bytes instead of the first 4 doesn't help; when I said that the first 4 bytes of a GUID are not guaranteed to be unique, what I meant was that no 4 bytes of a GUID are supposed to be unique; the entire 16 byte GUID is, but not any smaller part of it. You've actually made things worse with your change, because for version 4 GUIDs (used by .NET on Windows) the second 4 bytes include 4 non-random bits with fixed values; your edit has reduced the number of possible seed values to the low hundreds of millions.

[Glenn Slayden]

Ok, thanks. I reverted the change, and people can heed your comments if they have the concerns that you've raised.

[Agustin Garzon]

This is gold. I'm the lucky one in getting the ZERO result from the Random object

[Alex]

As mentioned in the comment earlier, this approach doesn't actually work when used from multiple threads. _local can't be instantiated in the constructor.

[Anthony Nichols]

Worst thing you can do is like to outside articles to give an answer and NOT actually include the answer... Especially when it is as simple as this.

[g.pickardou]

This is Solution is using the double checking anti pattern, also can introduce the initial error just less likelihood

[BlueRaja - Danny Pflughoeft]

@g.pickardou: Yep, you're right, I can't believe I never noticed that. The issue comes from the original article's code. It should be fixed now.

[Siavash Mortazavi]

I think you can replace the call to GenerateSeed() in Random constructor with: Guid.NewGuid().GetHashCode())

[NickL]

@BlueRaja-DannyPflughoeft this still isn't what a lot of people would call thread-safe. It's OK if you initialise it every time you try and use it, but if you initialise it once and then share that instance it will throw null reference exceptions.

[NickL]

@BlueRaja-DannyPflughoeft That's only true if you create a new ThreadSafeRandom on each new thread, because the work is being done in the constructor. Try running: var r = new ThreadSafeRandom(); await Task.Run(() => r.Next());

[Austin Salgat]

@BlueRaja-DannyPflughoeft Double checked locking (which is still in the example provided) is not thread safe because of instruction reordering inside the cpu. In this case _local can be assigned to before the constructor is called on the instance since _local is being read outside the lock. This is not an issue with Microsoft's x86 implementation of the CLR due to them adding memory barriers around reference assignment but on other platforms that may not be guaranteed since the C# standard doesn't enforce those memory barriers.

[BlueRaja - Danny Pflughoeft]

@Salgat: That hasn't been true since before .Net 2.0. See this question

[Austin Salgat]

@BlueRaja-DannyPflughoeft Reread my comment, specifically "This is not an issue with Microsoft's x86 implementation of the CLR". He even goes on to mention: "Note that Duffy also mentions that this is an MS-specific guarantee - it's not part of the ECMA spec (at least as of the the article's writing in 2006). So, Mono might not be as nice." which is what I also said in my comment.

[Ohad Schneider]

Why do you need the double lock? _local is thread-static, so no other thread could have initialized it while you were waiting for the lock... By the same logic, _local = new Random(seed); can be out of the lock. Indeed, the current .NET core implementation does both:github.com/dotnet/runtime/blob/master/src/libraries/…

[Magnus]

"they will have the same seeds" This has been fixed in .Net Core.

[Sandor Drieënhuizen]

For those interested, I ran a benchmark (using BenchmarkDotNet on Core 3.1) to compare random.Next() to threadSafeRandom.Next(), both using a shared random object. The thread-safe random turns out to take about 2.25x more time. Random: 6.479 ns | 0.0733 ns | 0.0686 ns. ThreadSafeRandom: 14.583 ns | 0.0934 ns | 0.0828 ns (numbers are Mean/Error/StdDev). Not bad.

[Wouter]

you could replace the bitconverter with guid.newguid().gethashcode()

[Wouter]

This looks like a great solution but I've some questions why use BitConverter.ToUInt32 vs BitConverter.ToInt32 it would cleanup the GetNext()? And why make the pool static? It might save you from multiple pools but in concurrent systems where you have many SafeRandom instances it might also become a bottle neck. How to seed the RNGCryptoServiceProvider?

[Theodor Zoulias]

For extra style points you could have used a Lazy<Random> GlobalRandom, to avoid the explicit lock. 😃

[Ohad Schneider]

@TheodorZoulias not sure I follow, the problem is not just initialization - if we drop the global lock we could have 2 threads accessing the global Random at the same time...

[Theodor Zoulias]

Ohad you are right. I don't know what I was thinking. The Lazy<T> class offers nothing in this case.

[Theodor Zoulias]

Upvoted for the idea, but I don't think that it's practical in general. Essentially you construct an object having a single method, and then return this method.

[JJ_Coder4Hire]

locking introduces performance issues. if you need a random for each thread, then just declare it in the thread and use it within the threads scope. lock free!

[BlueRaja - Danny Pflughoeft]

@JJ_Coder4Hire no, locking is extremely cheap unless you get stuck waiting on the lock (which should be rare here). Your idea does not work because of this comment