Is The Following VB.NET Implementation Of Random Thread-Safe?

Question

I know there are many thread-safety questions especially regarding the Random class in VB.NET and C#. I've read through several as well as a blog post from Jon Skeet (https://codeblog.jonskeet.uk/2009/11/04/revisiting-randomness/); yet I'm still having a difficult time assessing the thread safety of my piece of code. I apologize a priori for adding yet another question of this form.

My code is based off of the aforementioned blog post, but I attempted to implement the comments from Andrew Shapira near the bottom of the post.

Background: Any code that calls this class (directly or indirectly) is typically ran on multiple threads without the use of any type of thread-locking. While I understand I can pass the onus of thread safety to any calling code, I would prefer to make this class handle multiple threads on its own.

Option Explicit On
Option Strict On
Imports System.Threading

Public NotInheritable Class ThreadSafeRandom

    Private Shared globalSeed As Integer = 0

    Private Shared Function CreateRandom(ByVal initialSeed As Integer) As Random

        Dim newRandom As Random

        If initialSeed <> 0 Then
            newRandom = New Random(initialSeed)
            Return newRandom
        End If

        Dim seed As Integer = CInt(Now.Ticks And &HFFFF)

        Interlocked.CompareExchange(globalSeed, seed, 0)

        newRandom = New Random(globalSeed)

        Interlocked.Increment(globalSeed)

        Return newRandom
    End Function

    Public Shared Function RandomInteger(ByVal lowerBound As Integer,
                                         ByVal exclusiveUpperBound As Integer,
                                         Optional ByVal seed As Integer = 0) As Integer

        Dim newRandom As Random = CreateRandom(seed)
        Return newRandom.Next(lowerBound, exclusiveUpperBound)
    End Function
End Class

Explanation: globalSeed is the value that gets passed into the Random constructor by default. Every time it gets passed, it gets incremented via System.Threading.Interlocked in the CreateRandom function.

CreateRandom is the Private function that is called within the Public function RandomInteger. The dual purpose of CreateRandom is to assure that the same seed value is never passed while at the same time allows for a new Random object to always be created thus avoiding potential thread issues.

Lastly, in order to allow a user to replicate results, the class allows for user-inputted seed values.

Question(s): Is ThreadSafeRandom truly thread safe? If not, is it possible to implement it in thread-safe way without completely changing the implementation?

Answer 1

You ask if your implementation is thread-safe. I'd say that, in some sense of the term, yes it is. But careful here. What is this thing you call "thread safe"?. I'd agree the code is thread-safe in the sense that the program will run safely without corrupting data structures. But will the code do what you want and need (including meet goals you may not realize you have) under concurrent scenarios? That's debatable.

I think the first thing I would worry about with the code you posted is whether it's actually useful. Three major issues stand out to me:

1. A caller wanting to use a single seed value and get a sequence of random numbers in a given range is out of luck. They will get a whole new Random instance with each call to RandomInteger(), and that instance will always return the same number.
2. You're only using 16 bits of the Tick value. This seems like a needless reduction of entropy. Why not use &H7FFFFFFF?
3. There is not actually anything stopping two different threads from getting the same seed (which seemed to be a goal of yours).
4. Creating an instance of Random is not inexpensive. So this approach to thread-safety is very costly. I'm sure compared to just using lock, the latter would win hands-down.

On top of all of that, there is of course the problem of creating a sequence of Random objects by seeding them with an incrementing value. See the various articles cited in Jon's comment and also below.

Based on the valuable discussion found in those references, I would say that there are two obvious, simple, and useful ways to approach the problem, depending on what you need.

If each thread operates completely independently from each other, and thus any correlation between multiple instance of Random will be irrelevant, then the ThreadLocal<T> or [ThreadStatic] approaches should be fine. E.g.:

private static readonly ThreadLocal<Random> _threadRandom =
    new ThreadLocal<Random>(() => new Random());
public static Random Instance { get { return _threadRandom.Value; } }

or…

[ThreadStatic]
private static Random _threadRandom;

public static Random Instance
{
    get
    {
        if (_threadRandom == null)
        {
            _threadRandom = new Random();
        }

        return _threadRandom;
    }
}

If you need for multiple threads to operate together in a way that correlated sequences of random numbers would be a problem, then you should just use a single Random instance with a lock:

static class SharedRandom
{
    private static readonly Random _random = new Random();

    public static int Next()
    {
        lock (_random)
        {
            return _random.Next();
        }
    }

    // etc...wrap each `Random` method you need, as above
}

Useful references (you know about all of these…I'm just putting them here all in one place for convenience, and because a couple were in the comments, which are ephemeral):
Seeding Multiple Random Number Generators
C# in Depth: Random numbers
How do I seed a random class to avoid getting duplicate random values
Jon Skeet's coding blog: Revisiting randomness