53

I've written myself a multi-threaded random generator

public static class MyRandGen
{
    private static Random GlobalRandom = new Random();
    [ThreadStatic]
    private static Random ThreadRandom = new Random(SeedInitializer());
    private static int SeedInitializer()
    {
        lock (GlobalRandom) return GlobalRandom.Next();
    }

    public static int Next()
    {
        return ThreadRandom.Next();
    }
}

However, it throws me a NullReferenceException on firing Next(), which I don't understand. Is that kind of initializing ThreadStatic fields forbidden somehow?

I know I could just check if the field's initialized every time, but that's not the solution I'm looking for.

Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
Tarec
  • 3,268
  • 4
  • 30
  • 47
  • Why not use [`Lazy` it has thread safe options](http://msdn.microsoft.com/en-us/library/ee808725.aspx) – Mgetz Aug 06 '13 at 17:06
  • Your code works without an exception for me. VS2010\4.0 – Martin Aug 06 '13 at 17:07
  • Why not use `Rngcryptoserviceprovider` which is ThreadSafe – Sriram Sakthivel Aug 06 '13 at 17:07
  • I'm looking for a method, that'd be faster than simple locking - Rngcryptoserciveprovider is few times slower than that. I'll learn about the Lazy, thought I still don't know why doesn't my method work. – Tarec Aug 06 '13 at 17:19

2 Answers2

82

Initializing ThreadStatic fields is a little tricky. In particular there is this caveat:

Do not specify initial values for fields marked with ThreadStaticAttribute, because such initialization occurs only once, when the class constructor executes, and therefore affects only one thread.

in the MSDN Docs. What this means is that the thread running when the class is initialized gets that initial value you've defined in the field declaration, but all other threads will have a value of null. I think this is why your code is exhibiting the undesirable behavior described in your question.

A fuller explanation is in this blog.

(a snippet from the blog)

[ThreadStatic]
private static string Foo = "the foo string";

The ThreadStatic is initialized in the static constructor - which only executes once. So only the very first thread is assigned "the foo string" when the static constructor executes. When accessed in all subsequent threads, Foo is left at the uninitalized null value.

The best way to work around this is to use a property to access the Foo prop.

[ThreadStatic]
private static string _foo;

public static string Foo {
   get {
     if (_foo == null) {
         _foo = "the foo string";
     }
     return _foo;
   }
}

Note that there is no need for a lock in the static property, because each thread is acting upon the _foo that is just for that thread. There can't be contention with other threads. This is covered in this question: ThreadStatic and Synchronization

Community
  • 1
  • 1
  • That's the answer I've been looking for. I thought I could avoid null checking. Thank you for your help. – Tarec Aug 06 '13 at 17:25
  • Is that getter thread safe? If you had two methods with an affinity for the same thread, and they both called the getter at the same time, couldn't that cause a potential problem. Shouldn't there be a lock? – cost Sep 13 '14 at 20:49
  • 3
    @cost - Each thread has its own `_foo`, and the same thread cannot be accessing the getter more than once at the same time. See http://stackoverflow.com/questions/1087599/is-this-a-thread-safe-way-to-initialize-a-threadstatic . Also MSFT says `Any public static members of this type are thread safe` http://msdn.microsoft.com/en-us/library/system.threadstaticattribute(v=vs.110).aspx – hatchet - done with SOverflow Sep 13 '14 at 21:22
  • I don't see on either of those linked pages where it says the getter can only be accessed once at a time. The MSDN article looks like it's talking about the variable itself being thread safe, not the getter. The SO question looks like it ignores the possibility that two different program flows could be accessing the getter from the same thread via a very well timed context switch – cost Sep 13 '14 at 22:33
  • 4
    @cost - sure, two different threads can hit the getter at the same time. But each thread is accessing its own `_foo` that exists only for that thread (that's what ThreadStatic does), so there is no need to lock to prevent them stepping on `_foo`. They aren't accessing the same `_foo`. http://stackoverflow.com/questions/11507881/threadstatic-and-synchronization . As for two different program flows from the same thread...I'm not aware of the feature of context switching between multiple program flows within the same thread. – hatchet - done with SOverflow Sep 14 '14 at 05:02
  • I'm talking about two methods on the same thread hitting the getter at the same time. Imagine two separate timers that are both firing to make things happen on the UI thread, both accessing the same getter. If you have two methods that want to be called independently on the same thread, how does .NET handle that? – cost Sep 14 '14 at 07:46
  • I mean, if one of those methods ran for a really long time, would the other have to wait for it to finish first? – cost Sep 14 '14 at 07:55
  • 6
    @cost - A thread can't be interrupted by itself. As long as the GUI thread is in the getter, a windows timer in the GUI thread will be blocked. The only way I know that a single thread could enter the getter a second time before exiting the first would be if you did it intentionally by putting a DoEvents call within the getter or writing the getter to be recursive. Even in those cases though, a lock would not help. A lock only blocks other threads. A single thread can obtain multiple locks on the same object without blocking itself. This allows their use in re-entrant code. – hatchet - done with SOverflow Sep 14 '14 at 13:25
4

Previous answer is correct as to the reason for the issue.

if you can use .NET 4 or higher, use ThreadLocal instead as is built with an initializer.

See ThreadStatic v.s. ThreadLocal<T>: is generic better than attribute?

Then you don't need the accessor overload or null check on every read.

David Burg
  • 1,064
  • 12
  • 14