Is this StaticRandom class thread-safe?

Question

The program I am writing is a game server which will have multiple sessions in separate thread. To avoid unnecessary lock waiting time, [ThreadStatic] was added.

The program will include some async functions and ThreadSafeStaticRNG(this class) will be used in async.

public class ThreadSafeStaticRNG 
{
    [ThreadStatic]
    static Random r = new Random();

    /// <summary>
    /// get random number from [min,max)
    /// </summary>
    /// <returns></returns>
    public static int GetNext(int min, int max)
    {
        int n;
        lock (r)
        {
            n = r.Next(min, max);
        }
        return n;
    }
}

I need verification for;

1. As I understand, worker thread created with async/Task will not have separate ThreadStatic instance. So lock is necessary.
2. If above is true, only restriction in lock is that await cannot be used in lock. So it is safe to use lock in async function.
3. (edit) lock (r) for r instance, is it okay? Official document and other codes I looked created other object for only lock.

Answer 1

Code shown is (excessively) thread-safe and not correct (can generate same/synchronized random values).

ThreadStatic guarantees that each thread (either created by hand of taken from thread pool) has its own instance of the variable. As result you don't need to lock as long as you don't expose that variable outside (as shown in your code).

Each thread at each moment of time can only run one function - whether it is part of async method or function passed to thread. Thread will not switch between functions in the middle of the execution so there is no way two calls to GetNext will be made on the same thread in parallel at the same time.

Note that creating multiple Random objects quickly can lead to Random number generator only generating one random number - check this particular answer for proper initialization of such per-thread instances Random (using incremental seed).

Answer 2

1. The Task/Async might not have a seperate Thread assigned to it, and a Task might be executed on multiple threads.
2. Yes its safe to use Lock, and with the non-thread-safe Random function, neccessary to do so. There is no need to mix Lock and ThreadStatic. Its more obvious in your intent to use Lock() in my opinion, and leave off ThreadStatic.
3. Its normal to create a new Object and lock on it, rather than a lock on the operational object,but in your case its not incorrect to do so.

Perhaps a better pattern would be to invert the problem and get the caller to pass in an instance of a new Random() object into the code which depends on it. This would guarantee a unique instance of Random for each task, and also be more declarative and testable (passing in IRandom and using a test substitute in unit-test land would be the best overall solution.) - for unit testing you need predictible results, not random results, so the ability to substitute a known-number generator for a random-number generator via IOC would be important.