Is Object Assignment Thread Safe?

Question

Is this thread safe? Specifically, is it possible for the GetMyObject() method to return null? I understand it is possible for two threads to get a different instance of MyObject but I don't care about that. I just want to make sure it is safe to assume GetMyObject() will never return null.

   class Foo {
        private static MyObject obj;
        public static MyObject GetMyObject() {
            MyObject o = obj;
            if(o == null) {
                o = new MyObject();
                obj = o;
            }
            return o;
        }
        public static void ClearMyObject() {
            obj = null;
        }
    }

    class MyObject {}

Answer 1

Is this thread safe?

No.

is it possible for the GetMyObject() method to return null?

No.

The method is guaranteed to never return null. And all the reads and writes are guaranteed to be atomic. However, threads are not guaranteed to read the latest version of static field obj, and threads are not guaranteed to have a consistent view of the sequence of changes to obj. Arbitrarily many threads may race and observe different values of obj. I would not consider this code "thread safe" as a result, but maybe you have a different definition of "thread safe". That's the problem with asking this question; there's no standard definition of the term that everyone reliably agrees upon.

Answer 2

GetMyObject() can never return null. The easy way to see this is to note that 'o' is a local variable so no-one else can affect it.

Answer 3

Well, let's reason through this:

public static MyObject GetMyObject() {
        MyObject o = obj;
        if(o == null) {
            o = new MyObject();
            obj = o;
        }
        return o;

}

There is only one return statement. The only way that this method can produce a null return value is if that sole return statement return o has o == null being true when it is executed.

If o is null when return o is executed, that means we came out of the if block with o as null. The only way we can come out of the o block with o as null is if o == null was true when the conditional for the if block was tested (if o == null is false, then o != null is true and since o is a local variable it can not be affected by any other thread. But then o == null being true implies we end up inside the if block and now when the constructor invocation o = new MyObject() returns, we are guaranteed that o is not null. The second statement in the if block, obj = o, does not affect the value of o. Again, since o is a local variable, it doesn't matter if there are multiple threads burning through this code path: each thread has its own o and no other thread can touch any other thread's o.

Therefore, whether or not o == null is true or false, we end up with o == null being false when the if block completes.

Therefore, this method is guaranteed to return a non-null value.

I just want to make sure it is safe to assume GetMyObject() will never return null.

Well, that's fine if that's all you care about. But let's be clear about something. Your method is NOT thread safe. It is perfectly possible for two instances of MyObject to be constructed and two different callers could end up seeing different return values even though it is clear that you intention is to only have one. To fix this, I recommend just using Lazy<T>:

private static Lazy<MyObject> obj;

static Foo() {
    obj = new Lazy<MyObject>(
        () => new MyObject(),
        true
    );
}

public static MyObject GetMyObject() {
    return obj.Value;
}

public static void ClearMyObject() {
    obj = new Lazy<MyObject>(
        () => new MyObject(), 
        true
    );
}

Answer 4

It won't return null, but it is by no means thread-safe by most accepted definitions. Presumably, you want to store your object into shared state and have that accessed by other threads. In this case, other threads may create their own copies (as you said) and try to store them, but all threads are not guaranteed to see the latest version of that object (or any other thread's version of that object). Likewise, your ClearMyObject() method will not do what you think it will.

Use Lazy<T> instead which will provide what you're looking for.

public static readonly Lazy<MyObject> myObject = new Lazy<MyObject>(() => new MyObject(), true);