Can a readonly field in .NET become null ?

Question

I have a strange bug in a multithreaded app:

public class MyClass
{
   private readonly Hashtable HashPrefs;
   public MyClass(int id)
  {
     HashPrefs = new Hashtable();
  } 

  public void SomeMethodCalledFromAnotherThread(string hashKey,string hashValue)
 {
    if (HashPrefs.Contains(hashKey))  // <-- throws NullReferenceException
    {

    }
 }
}

One thread does the :

 SomeQueue.Add(new MyClass(1));

And another thread does the :

 SomeQueue.Dequeue().SomeMethodCalledFromAnotherThread(SomeClass.SomeMethod(),"const value");

But how can the second thread call the method before the constructor is finished?

Edit: I added the part with the function parameters, as it seems this might be relevant. As far as I can tell, the hashKey being passed on cannot be null, as SomeMethod() always returns a relevant string.

As others have pointed out,if the problem was a null haskKey parameter passed to the Contains() , the exception would be ArgumentNullException.

Answer 1

Yes, readonly fields can be accessed via reflection and their values changed. So the answer to your question is yes, it is possible.

However there are many other things that could also be creating issues for you. Multi-threaded code is difficult to write and the problems that can arise are difficult to diagnose.

---

As a side note, are you sure that you aren't getting an ArgumentNullException? If somekey is null, the Hashtable.Contains method will throw an ArgumentNullException. Perhaps that is the issue?

Answer 2

Are you sure that it's HashPrefs.Contains that is throwing the exception, and not whatever "somekey" represents? If you're not, could you please post the full exception details, as returned by its ToString method?

Answer 3

First of all, the other thread cannot access the instance until the constructor is complete, since the instance itself won't be assigned until the constructor is finished. That being said, the other thread could access the variable that holds the instance before the constructor completes, but it would be null at that time. That would generate the NullReferenceException, but it would come from the method accessing the instance, not from the method inside the instance.

All of that being said, the only real solution that I can see to this is to use an external lock that synchronizes the code around retrieving the instance. If you're accessing a Hashset (or any other type where instance member's aren't guaranteed to be thread safe) you'll need to perform locking around the operations anyhow.

Consider the following class as the one containing a reference to MyClass and the creator of the other threads:

public class MasterClass
{
    private MyClass myClass;
    private object syncRoot = new object(); // this is what we'll use to synchronize the code

    public void Thread1Proc()
    {
        lock(syncRoot)
        {
            myClass = new MyClass();
        }
    }

    public void Thread2Proc()
    {
        lock(syncRoot)
        {
            myClas.SomeMethodCalledFromAnotherThread();
        }
    }
}

This is a very (possibly overly) simplistic view of how the synchronization should take place, but the general idea of this approach is to wrap all code that interacts with the shared object inside the lock block. You truly should do some studying on how to write multithreaded processes, and the various approaches aimed at sharing resources among multiple threads.

EDIT

A completely different possibility is that the values of the instance variable are being cached in a thread-specific manner. If you aren't locking (which creates a memory barrier), then try marking the instance variable as volatile, which will ensure that the reads and writes occur in the proper order.

Answer 4

Just do

HashPrefs = Hashtable.Synchronized(new Hashtable());

No need to reinvent the wheel.

Answer 5

Reflection is one way to do this (SetField).

A second way to get this curiosity is if you give away the reference too early, by passing this (or sometihng involving an implicit this, such as a field captured into an anonymous method / lambda) out of the .ctor:

public MyClass(int id)
{
    Program.Test(this); // oopsie ;-p
    HashPrefs = new Hashtable();
}

Or more likely (given the question):

SomeQueue.Add(this);

Another relevant question might be - can it not be assigned in the first place? And the answer to that is yes, especially if you are using serialization. Contrary to popular belief, you can actually bypass the constructors, if you have a reason; DataContractSerializer is a good example of something that does this...

The following will create a MyClass with a null field:

MyClass obj = (MyClass)
    System.Runtime.Serialization.FormatterServices.GetUninitializedObject(
        typeof(MyClass));

Answer 6

Several things you can try:

1. Try initializing the Hashtable as a member variable (outside of the constructor), and with public property of the same name, see if someone is calling it to make an assignment, like so:

   public class MyClass {
   private readonly Hashtable hashPrefs = new Hashtable();
   
   
   public MyClass(int id)
   {
   
   
   }
   
   
   public Hashtable HashPrefs
   {
       set
       {
           throw new InvalidOperationException("This shouldn't happen");
       }
   }
   

   }

2. Secondly, what type is "SomeQueue"? Is that just a regular List<>? Or is it some special self-implemented Queue that relies on some XML/binary serialization? If so, have you marked HashPrefs as [Serializable]? Or as a [DataMember]?

Answer 7

Are you absolutely sure that HashPrefs.Contains is throwing the NPE? I would think SomeQueue.Dequeue().SomeMethodCalledFromAnotherThread() would be the more likely candidate... because chances are the constructor and Add operations haven't finished yet when you try to Dequeue the element.

In fact, it might not be a bad idea to check SomeQueue.Count* before Dequeueing an element.

Edit: Or even better, null check before calling SomeMethodCalledFromAnotherThread, assuming that your other thread is checking the queue in a loop... because if it's not, lock the queue prior to the Add and Dequeue operations.

MyClass mine = SomeQueue.Dequeue();

if (null != mine)
    mine.SomeMethodCalledFromAnotherThread();

*An extension method added in .NET 3.5.

Answer 8

A couple ideas come to mind:

• The hashtable is being concurrently modified leading to an inconsistent internal state.
• The call to contains performs equality checks which attempt to dereference a value that has a null field. What type is `somekey`?

Answer 9

Your instructions might be reordered so that a reference to an instance of a MyClass is added to the queue before it is fully constructed. Here's an article about double-checked locking that sort of touches on this topic. From the article:

Vance, a devlead on the CLR JIT team explained that the issue is around the CLR memory model… Essentially the memory model allows for non-volatile reads\writes to be reordered as long as that change can not be noticed from the point of view of a single thread.

Take a look at System.Threading.Thread.MemoryBarrier. You might need to do something like this:

 MyClass temp = new MyClass(1);
 System.Threading.Thread.MemoryBarrier();
 SomeQueue.Add(temp);

The MemoryBarrier ensures that all the instructions before it are executed before proceeding.