HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Singleton SpinLock: Making Random Thread-Safe

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomspinlocksingletonthreadsafemaking

Problem

Is this a valid and safe use of .NET's System.Threading.SpinLock?


Why am I doing this?


Random's public methods are not thread-safe. I could be calling from
any thread which I do not know until run-time; they are from the
Thread Pool, and it could be called fairly frequently (up to a hundred
times a second), so I would rather not create a new Random object each
time.

public static class SingleRandom
{
    private static Random random;
    private static SpinLock spinLock;

    static SingleRandom()
    {
        random = new Random();
        spinLock = new SpinLock();
    }

    public static int Next()
    {
        bool gotLock = false;
        spinLock.Enter(ref gotLock);
        int rv = random.Next();
        if(gotLock)
            spinLock.Exit(false); // ASSUMPTION not IA64, TODO what if ARM?
        return rv;
    }

    public static int Next(int min, int max)
    {
        bool gotLock = false;
        spinLock.Enter(ref gotLock);
        int rv = random.Next(min, max);
        if (gotLock)
            spinLock.Exit(false); // ASSUMPTION not IA64, TODO what if ARM?
        return rv;
    }

    public static double NextDouble()
    {
        bool gotLock = false;
        spinLock.Enter(ref gotLock);
        double rv = random.NextDouble();
        if (gotLock)
            spinLock.Exit(false); // ASSUMPTION not IA64, TODO what if ARM?
        return rv;
    }
}

Solution


  • SingleRandom is a really bad name for your class. Single means float in C# context, so your name implies that its a Random which generates float values, which is not true. ThreadSafeRandom or SynchronizedRandom are the examples of better naming.



-
Are you sure that using SpinLock in you case improves performance in any way? Somehow i think that a simple lock will be just as fast, while being a lot more readable.

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


-
I am not sure that making your class static is the best way to go. In my experience in such cases its almost always better to make a non-static class and then to either inject its instance or initialize some static readonly field.

Code Snippets

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

Context

StackExchange Code Review Q#36542, answer score: 6

Revisions (0)

No revisions yet.