Anything wrong with this way to implement atomic doubles/longs/datetimes/nullables?

You can't declare a double, long, DateTime, any nullable or any other structs as volatile (it wouldn't work if you could because writes aren't atomic), but in my particular case I need a volatlie, atomically-written DateTime?.

I wrote this simple class that ensures that writes are atomic. If you take a copy as below, it will always have either the value from before the write or the value from after the write, never any incomplete values.

/// <summary>
/// A T? where writes are atomic. Implemented as a class (which always has atomic read/writes) containing a readonly value.
/// </summary>
public class AtomicNullable<T> where T: struct {
    public readonly T Value;

    public AtomicNullable(T value) {
        this.Value = value;
    }

    public static implicit operator AtomicNullable<T>(T value) {
        return new AtomicNullable<T>(value);
    }
}

Usage:

private volatile AtomicNullable<DateTime> expiryTime = null;

private bool IsExpired() {
    // Copy of expiry makes sure it doesn't get set from another thread in the middle of evaluating the boolean expression.
    AtomicNullable<DateTime> expiry = this.expiryTime;
    return expiry == null
        || expiry开发者_高级运维.Value < DateTime.UtcNow;
}


private void Calculate() {
    if (IsExpired()) {
        lock (locker) {
            if (IsExpired()) {
                // do calculation...
                expiryTime = DateTime.UtcNow + MaximumCachedObjectAge;
            }
        }
    }
}

---

It looks like you have reinvented boxing (except with greater type-safety).

private volatile object expiryTime = null;

private bool IsExpired()
{
    object expiry = this.expiryTime;
    return expiry == null
        || (DateTime)expiry < DateTime.UtcNow;
}

The type-safety thing is nice, though.

These are the things that I would change:

Calculate() should be CalculateIfExpired() and it should call Calculate() to do the real work.

Currently, Calculate is messing around with setting the expiryTime field. Why should it know how to set expiryTime when it doesn't know how to read expiryTime? Instead, IsExpired() should have a nice, little SetExpired() sitting next to it on your tool shelf. And the code should pretend that expiryTime is only in scope in those two methods (or make another class so it doesn't have to pretend).

And now to finally answer your question :-)

I agree with @Eric Lippert that basic locking is better than double-checked locking unless it is shown to be not good enough. I think the double-checked locking is OK, though, so long as you never forget to mark the controlling variable as volatile. All of the problems with this method that I have seen assume that the variable is not volatile.