Is C开发者_JS百科#'s Random.Next() method thread safe?

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.

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.

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.)

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.

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.

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

As was written before, anwer is No. However, starting from .NET6, we have out of box thread-safe alternative: Random.Shared.Next();

See details here: https://learn.microsoft.com/en-us/dotnet/api/system.random.shared?view=net-6.0

For what its worth, here is a thread-safe, cryptographically strong RNG that inherits Random.

The implementation includes static entry points for ease of use, they have the same names as the public instance methods but are prefixed with "Get".

A call to the RNGCryptoServiceProvider.GetBytes is a relatively expensive operation. This is mitigated through the use of an internal buffer or "Pool" to make less frequent, and more efficient use of RNGCryptoServiceProvider. If there are few generations in an application domain then this could be viewed as overhead.

using System;
using System.Security.Cryptography;

public class SafeRandom : Random
{
    private const int PoolSize = 2048;

    private static readonly Lazy<RandomNumberGenerator> Rng =
        new Lazy<RandomNumberGenerator>(() => new RNGCryptoServiceProvider());

    private static readonly Lazy<object> PositionLock =
        new Lazy<object>(() => new object());

    private static readonly Lazy<byte[]> Pool =
        new Lazy<byte[]>(() => GeneratePool(new byte[PoolSize]));

    private static int bufferPosition;

    public static int GetNext()
    {
        while (true)
        {
            var result = (int)(GetRandomUInt32() & int.MaxValue);

            if (result != int.MaxValue)
            {
                return result;
            }
        }
    }

    public static int GetNext(int maxValue)
    {
        if (maxValue < 1)
        {
            throw new ArgumentException(
                "Must be greater than zero.",
                "maxValue");
        }
        return GetNext(0, maxValue);
    }

    public static int GetNext(int minValue, int maxValue)
    {
        const long Max = 1 + (long)uint.MaxValue;

        if (minValue >= maxValue)
        {
            throw new ArgumentException(
                "minValue is greater than or equal to maxValue");
        }

        long diff = maxValue - minValue;
        var limit = Max - (Max % diff);

        while (true)
        {
            var rand = GetRandomUInt32();
            if (rand < limit)
            {
                return (int)(minValue + (rand % diff));
            }
        }
    }

    public static void GetNextBytes(byte[] buffer)
    {
        if (buffer == null)
        {
            throw new ArgumentNullException("buffer");
        }

        if (buffer.Length < PoolSize)
        {
            lock (PositionLock.Value)
            {
                if ((PoolSize - bufferPosition) < buffer.Length)
                {
                    GeneratePool(Pool.Value);
                }

                Buffer.BlockCopy(
                    Pool.Value,
                    bufferPosition,
                    buffer,
                    0,
                    buffer.Length);
                bufferPosition += buffer.Length;
            }
        }
        else
        {
            Rng.Value.GetBytes(buffer);
        }
    }

    public static double GetNextDouble()
    {
        return GetRandomUInt32() / (1.0 + uint.MaxValue);
    }

    public override int Next()
    {
        return GetNext();
    }

    public override int Next(int maxValue)
    {
        return GetNext(0, maxValue);
    }

    public override int Next(int minValue, int maxValue)
    {
        return GetNext(minValue, maxValue);
    }

    public override void NextBytes(byte[] buffer)
    {
        GetNextBytes(buffer);
    }

    public override double NextDouble()
    {
        return GetNextDouble();
    }

    private static byte[] GeneratePool(byte[] buffer)
    {
        bufferPosition = 0;
        Rng.Value.GetBytes(buffer);
        return buffer;
    }

    private static uint GetRandomUInt32()
    {
        uint result;
        lock (PositionLock.Value)
        {
            if ((PoolSize - bufferPosition) < sizeof(uint))
            {
                GeneratePool(Pool.Value)
            }

            result = BitConverter.ToUInt32(
                Pool.Value,
                bufferPosition);
            bufferPosition+= sizeof(uint);
        }

        return result;
    }
}

Since Random isn't thread-safe, you should have one per thread, rather than a global instance. If you're worried about these multiple Random classes being seeded at the same time (i.e. by DateTime.Now.Ticks or such), you can use Guids to seed each of them. The .NET Guid generator goes to considerable lengths to ensure non-repeatable results, hence:

var rnd = new Random(BitConverter.ToInt32(Guid.NewGuid().ToByteArray(), 0))

Reimplementation of BlueRaja's answer using ThreadLocal:

public static class ThreadSafeRandom
{
    private static readonly System.Random GlobalRandom = new Random();
    private static readonly ThreadLocal<Random> LocalRandom = new ThreadLocal<Random>(() => 
    {
        lock (GlobalRandom)
        {
            return new Random(GlobalRandom.Next());
        }
    });

    public static int Next(int min = 0, int max = Int32.MaxValue)
    {
        return LocalRandom.Value.Next(min, max);
    }
}

Per documentation

Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

http://msdn.microsoft.com/en-us/library/system.random.aspx

Here is a simple solution that doesn't involve creating classes. Hides everything inside an ad-hoc lambda:

private static Func<int, int, int> GetRandomFunc()
{
    Random random = new Random();
    object lockObject = new object();

    return (min, max) =>
    {
        lock (lockObject)
        {
            return random.Next(min, max);
        }
    };
}

In any class that needs a thread safe random generator define

private static readonly Func<int, int, int> GetRandomNext = GetRandomFunc();

Then use it freely inside your class:

int nextRandomValue = GetRandomNext(0, 10);

The radndom function can have different signatures depending what is needed. e.g.

private static Func<int> GetRandomFunc()
{
    Random random = new Random();
    object lockObject = new object();

    return () =>
    {
        lock (lockObject)
        {
            return random.Next();
        }
    };
}

For a thread safe random number generator look at RNGCryptoServiceProvider. From the docs:

Thread Safety

This type is thread safe.

I just created Random instance on a load of threads simultaneously and got unexpected patterns from the results because, presumably, all the seeds were the same.

My solution was to create a thread safe class -

  public class StaticRandom
    {
        static Random rnd = new Random();

        public static int GetNext()
        {
            // random is not properly thread safe
            lock(rnd)
            {
                return rnd.Next();
            }
        }
    }

And then, create a new Random object instance for each thread -

// Take this threads instance seed from the static global version
Random rnd = new Random(StaticRandom.GetNext());

Here's an efficient solution using ThreadLocal:

public class ThreadSafeRandom
{
    private static readonly Random _global = new Random();
    private static readonly ThreadLocal<Random> _local = new ThreadLocal<Random>(() =>
    {
        int seed;
        lock (_global)
        {
            seed = _global.Next();
        }
        return new Random(seed);
    });

    public static Random Instance => _local.Value;
}

To use, replace:

var random = new Random();
var myNum = random.Next();

With:

var myNum = ThreadSafeRandom.Instance.Next();

This solution has been packaged as a NuGet package with source available on GitHub.

EDIT: Since .NET 6.0 you can use Random.Shared.Next() instead. You can still use the above package which chooses between the above code or Random.Shared with preprocessor directives.

UPDATED: It is not. You need to either reuse an instance of Random on each consecutive call with locking some "semaphore" object while calling the .Next() method or use a new instance with a guaranteed random seed on each such call. You can get the guaranteed different seed by using cryptography in .NET as Yassir suggested.

The traditional thread local storage approach can be improved upon by using a lock-less algorithm for the seed. The following was shamelessly stolen from Java's algorithm (possibly even improving on it):

public static class RandomGen2 
{
    private static readonly ThreadLocal<Random> _rng = 
                       new ThreadLocal<Random>(() => new Random(GetUniqueSeed()));

    public static int Next() 
    { 
        return _rng.Value.Next(); 
    } 

    private const long SeedFactor = 1181783497276652981L;
    private static long _seed = 8682522807148012L;

    public static int GetUniqueSeed()
    {
        long next, current;
        do
        {
            current = Interlocked.Read(ref _seed);
            next = current * SeedFactor;
        } while (Interlocked.CompareExchange(ref _seed, next, current) != current);
        return (int)next ^ Environment.TickCount;
   } 
}