Is the null coalesce operator thread safe?

Question

So this is the meat of the question: Can Foo.Bar ever return null? To clarify, can '_bar' be set to null after it's evaluated as non-null and before it's value is returned?

    public class Foo
    {
        Object _bar;
        public Object Bar
        {
            get { return _bar ?? new Object(); }
            set { _bar = value; }
        }
    }

I know using the following get method is not safe, and can return a null value:

            get { return _bar != null ? _bar : new Object(); }

UPDATE:

Another way to look at the same problem, this example might be more clear:

        public static T GetValue<T>(ref T value) where T : class, new()
        {
            return value ?? new T();
        }

And again asking can GetValue(...) ever return null? Depending on your definition this may or may not be thread-safe... I guess the right problem statement is asking if it is an atomic operation on value... David Yaw has defined the question best by saying is the above function the equivalent to the following:

        public static T GetValue<T>(ref T value) where T : class, new()
        {
            T result = value;
            if (result != null)
                return result;
            else
                return new T();
        }

Answer 1

No, this is not thread safe.

The IL for the above compiles to:

.method public hidebysig specialname instance object get_Bar() cil managed
{
    .maxstack 2
    .locals init (
        [0] object CS$1$0000)
    L_0000: nop 
    L_0001: ldarg.0 
    L_0002: ldfld object ConsoleApplication1.Program/MainClass::_bar
    L_0007: dup 
    L_0008: brtrue.s L_0010
    L_000a: pop 
    L_000b: newobj instance void [mscorlib]System.Object::.ctor()
    L_0010: stloc.0 
    L_0011: br.s L_0013
    L_0013: ldloc.0 
    L_0014: ret 
}

This effectively does a load of the _bar field, then checks its existence, and jumps ot the end. There is no synchronization in place, and since this is multiple IL instructions, it's possible for a secondary thread to cause a race condition - causing the returned object to differ from the one set.

It's much better to handle lazy instantiation via Lazy<T>. That provides a thread-safe, lazy instantiation pattern. Granted, the above code is not doing lazy instantiation (rather returning a new object every time until some time when _bar is set), but I suspect that's a bug, and not the intended behavior.

In addition, Lazy<T> makes setting difficult.

To duplicate the above behavior in a thread-safe manner would require explicit synchronization.

---

As to your update:

The getter for the Bar property could never return null.

Looking at the IL above, it _bar (via ldfld), then does a check to see if that object is not null using brtrue.s. If the object is not null, it jumps, copies the value of _bar from the execution stack to a local via stloc.0, and returns - returning _bar with a real value.

If _bar was unset, then it will pop it off the execution stack, and create a new object, which then gets stored and returned.

Either case prevents a null value from being returned. However, again, I wouldn't consider this thread-safe in general, since it's possible that a call to set happening at the same time as a call to get can cause different objects to be returned, and it's a race condition as which object instance gets returned (the set value, or a new object).

Answer 2

I wouldn't use the word 'thread safe' to refer to this. Instead, I would ask the question, which of these is the same as the null coalesce operator?

get { return _bar != null ? _bar : new Object(); }

or

get
{
    Object result = _bar;
    if(result == null)
    {
        result = new Object();
    }
    return result;
}

From reading the other responses, it looks like it compiles to the equivalent to the second, not the first. As you noted, the first could return null, but the second one never will.

Is this thread safe? Technically, no. After reading _bar, a different thread could modify _bar, and the getter would return a value that's out of date. But from how you asked the question, I think this is what you're looking for.

Edit: Here's a way to do this that avoids the whole problem. Since value is a local variable, it can't be changed behind the scenes.

public class Foo
{
    Object _bar = new Object();
    public Object Bar
    {
        get { return _bar; }
        set { _bar = value ?? new Object(); }
    }
}

Edit 2:

Here's the IL I see from a Release compile, with my interpretation of the IL.

.method public hidebysig specialname instance object get_Bar_NullCoalesce() cil managed
{
    .maxstack 8
    L_0000: ldarg.0                         // Load argument 0 onto the stack (I don't know what argument 0 is, I don't understand this statement.)
    L_0001: ldfld object CoalesceTest::_bar // Loads the reference to _bar onto the stack.
    L_0006: dup                             // duplicate the value on the stack.
    L_0007: brtrue.s L_000f                 // Jump to L_000f if the value on the stack is non-zero. 
                                            // I believe this consumes the value on the top of the stack, leaving the original result of ldfld as the only thing on the stack.
    L_0009: pop                             // remove the result of ldfld from the stack.
    L_000a: newobj instance void [mscorlib]System.Object::.ctor()
                                            // create a new object, put a reference to it on the stack.
    L_000f: ret                             // return whatever's on the top of the stack.
}

Here's what I see from the other ways of doing it:

.method public hidebysig specialname instance object get_Bar_IntermediateResultVar() cil managed
{
    .maxstack 1
    .locals init (
        [0] object result)
    L_0000: ldarg.0 
    L_0001: ldfld object CoalesceTest::_bar
    L_0006: stloc.0 
    L_0007: ldloc.0 
    L_0008: brtrue.s L_0010
    L_000a: newobj instance void [mscorlib]System.Object::.ctor()
    L_000f: stloc.0 
    L_0010: ldloc.0 
    L_0011: ret 
}

.method public hidebysig specialname instance object get_Bar_TrinaryOperator() cil managed
{
    .maxstack 8
    L_0000: ldarg.0 
    L_0001: ldfld object CoalesceTest::_bar
    L_0006: brtrue.s L_000e
    L_0008: newobj instance void [mscorlib]System.Object::.ctor()
    L_000d: ret 
    L_000e: ldarg.0 
    L_000f: ldfld object CoalesceTest::_bar
    L_0014: ret 
}

In the IL, it's obvious that it's reading the _bar field twice with the trinary operator, but only once with the null coalesce and the intermediate result var. In addition, the IL of the null coalesce method is very close to the intermediate result var method.

And here's the source I used to generate these:

public object Bar_NullCoalesce
{
    get { return this._bar ?? new Object(); }
}

public object Bar_IntermediateResultVar
{
    get
    {
        object result = this._bar;
        if (result == null) { result = new Object(); }
        return result;
    }
}

public object Bar_TrinaryOperator
{
    get { return this._bar != null ? this._bar : new Object(); }
}

Answer 3

The getter will never return null.

This is because when the read is performed on the variable (_bar) the expression is evaluated and the resulting object (or null) is then "free" of the variable (_bar). It is the result of this first evaluation which is then "passed" to the coalesce operator. (See Reed's good answer for the IL.)

However, this is not thread-safe and an assignment may easily be lost for the same reason as above.

Answer 4

Reflector says no:

List<int> l = null;
var x = l ?? new List<int>();

Compiles to:

[STAThread]
public static void Main(string[] args)
{
    List<int> list = null;
    if (list == null)
    {
        new List<int>();
    }
}

Which does not appear to be thread safe in the respect you've mentioned.