Implementing a Random provider in .Net 3.5

Question

Here is the culmination of a Skeet posting for a random provider:

public static class RandomProvider
{    
    private static int seed = Environment.TickCount;

    private static ThreadLocal<Random> randomWrapper = new ThreadLocal<Random>(() =>
        new Random(Interlocked.Increment(ref seed))
    );

    public static Random GetThreadRandom()
    {
        return randomWrapper.Value;
    }
}

I would like to use the same concept in a .NET 3.5 project, so ThreadLocal is not an option.

How would you modify the code to have a thread safe random provider without the help of ThreadLocal?

UPDATE

Ok, am going with Simon's [ThreadStatic] for now since I understand it the best. Lots of good info here to review and rethink as time allows. Thanks all!

public static class RandomProvider
{
    private static int _seed = Environment.TickCount;

    [ThreadStatic]
    private static Random _random;

    /// <summary>
    /// Gets the thread safe random.
    /// </summary>
    /// <returns></returns>
    public static Random GetThreadRandom() { return _random ?? (_random = new Random(Interlocked.Increment(ref _seed))); }
}

Pearlstein

There is also a follow up article: msmvps.com/blogs/jon_skeet/archive/2009/11/04/… where he discusses other thread safety strategies. – Phyliciaphylis

Answer 1

You can use the ThreadStaticAttribute

[ThreadStatic]
private static Random _random;

private static Random Random
{
    get
    {
        int seed = Environment.TickCount;

        return _random ?? (_random = new Random(Interlocked.Increment(ref seed)))
    }
}

Zulmazulu

Nice and worth +1, but sounds too easy, after reading Skeet's take on it. What advantage would the ThreadLocal implementation have over this? – Pearlstein

I didn't know about ThreadLocal prior to this question, but at first glance the only advantage I see is that ThreadLocal can be given a factory method to initialise the value, so you don't have to check for null – Zulmazulu

Skeets post seems to indicate that ThreadLocal is a bit faster, but in my own tests I could not reproduce that. I went with ThreadLocal for my random generator library, but unfortunately it uses code contracts, and thus only works on .net 4. – Phyliciaphylis

@CodeInChaos. Yeah, that f.u. post by Skeet was a great find. Seems to indicate ThreadStatic, even in 3.5. is a slightly better than locking. Thanks for your help on this one. – Pearlstein

Answer 2

How would you modify the code to have a thread safe random provider without the help of ThreadLocal?

Jon answers your question in the article you linked to:

use one instance, but also use a lock which every caller has to remember to acquire while they're using the random number generator. That can be simplified by using a wrapper which does the locking for you, but in a heavily multithreaded system you'll still potentially waste a lot of time waiting for locks.

So just lock it every time in the wrapper, and unlock it when you're done.

If that is cheap enough, great, it's cheap enough.

If that is not cheap enough then you have two choices. First, make it cheaper. Second, write a threadsafe implementation of a pseudo-random-number generator that can be used without locking.

There are a number of ways to make it cheaper. For example, you could trade space for time; you could generate an array of a hundred thousand random numbers when the program starts up, and then write a lock-free algorithm that proffers up previously-computed random values from the array. When you run out of values, generate another hundred thousand values in an array, and swap the new array for the old one.

That has the downside that its memory consumption is about a hundred thousand times larger than it could be, and that every hundred thousand numbers suddenly it gets really slow, and then speeds up again. If that's unacceptable then come up with a strategy that is acceptable. You're the one who knows what is acceptable performance and what isn't.

Or, like I said, write your own if you don't like the one that is provided for you. Write a threadsafe implementation of Random with acceptable performance and use it from multiple threads.

I saw what Jon said about locking in the wrapper but not sure how the code would look!

Something like:

sealed class SafeRandom
{
    private Random random = new Random();
    public int Next()
    {
        lock(random)
        {
            return random.Next();
        }
    }
}

Now every time you call Next, you take out a lock. (Always lock on a private object; that way you know that your code is the only code locking it!) If two threads call Next "at the same time" then the "loser" blocks until the "winner" leaves the Next method.

If you wanted, you could even make the SafeRandom object a static class:

static class SafeRandom
{
    private static Random random = new Random();
    public static int Next()
    {
        lock(random)
        {
            return random.Next();
        }
    }
}

and now you can call SafeRandom.Next() from any thread.

Methodist

Hmmm, like you know what you're talking about :--) – Pearlstein

Yes, I saw what Jon said about locking in the wrapper but not sure how the code would look! – Pearlstein

That was the part of the part I was missing, locking on Random itself as opposed to some object lock. The other part being that the provider class is not static in this implementation...yes? – Pearlstein

You could even make SafeRandom inherit from Random. – Phyliciaphylis

@Berryl: You could lock on another private object, but in this case, why bother? You have a suitable private object right there. The "random" field is not static; instead, you can now make a single SafeRandom instance and put it in a static field and use it from multiple threads. – Methodist

@Eric Lippert. That sounds right - will revist all of your answer 'in depth' later. Awesome answer(s)! – Pearlstein

@Eric: FxCop will sometimes complain when you lock on private objects. Jon Skeet explained here https://mcmap.net/q/1323887/-don-39-t-use-system-threading-timer-to-synchronize that the reason is that we don't know if the object locks on this and that you should never lock on anything but System.Object – Siple

Answer 3

I just read the link to "C# in Depth" and, although I agree to the fact that Random being not threadsafe is a pain, I would actually use a different approach to getting rid of the problem, namely for performance reason.

Instantiating a Random engine is quite a heavy action, so I'd rather keep only one instance and make it thread safe through the use of locks :

public static class RandomProvider 
{     
    private static Random randomEngine = new Random(Environment.TickCount);

    private static object randomLock = new object();

    public static int GetRandomValue() 
    { 
        lock(randomLock)
        {
            return randomEngine.Next();
        }
    } 
} 

HTH

Vaso

Did you mean to post different code? This looks like the original 4.0 code, no? – Pearlstein

No : it does not use ThreadLocal anymore, and does not use ThreadStatic either – Vaso