Что-то не так с этим способом реализации атомарных удваиваний / длинных / datetime / nullables? - PullRequest
Новая
       35

Что-то не так с этим способом реализации атомарных удваиваний / длинных / datetime / nullables?

3 голосов
/ 30 марта 2011

Вы не можете объявить double, long, DateTime, любые обнуляемые или любые другие структуры как volatile (это не будет работать, если вы могли бы, потому что записи не являются атомарными), но в В моем конкретном случае мне нужен волатли, написанный атомарно DateTime?.

Я написал этот простой класс, который гарантирует, что записи являются атомарными. Если вы возьмете копию, как показано ниже, она всегда будет иметь значение от до записи или значение от после записи, никогда не будет никаких неполных значений. 

/// <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);
    }
}

Использование: 

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;
            }
        }
    }
}

1 Ответ

3 голосов
/ 30 марта 2011

Похоже, что вы заново изобрели бокс (за исключением случаев, когда требуется повышенная безопасность типов). 

private volatile object expiryTime = null;

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

А вот с типобезопасностью все хорошо.

Вот вещи, которые я бы изменил:

Calculate() должно быть CalculateIfExpired() и должно вызывать Calculate() для выполнения реальной работы.

В настоящее время Calculate бездельничает с настройкой поля expiryTime. Почему он должен знать, как установить expiryTime, когда он не умеет читать expiryTime? Вместо этого IsExpired() должен иметь красивый маленький SetExpired() рядом с ним на полке для инструментов. И код должен притвориться, что expiryTime находится только в области действия этих двух методов (или создать другой класс, чтобы он не притворялся).

А теперь, наконец, чтобы ответить на ваш вопрос : -)

Я согласен с @Eric Lippert, что базовая блокировка лучше, чем блокировка с двойной проверкой, если только не показано, что она недостаточно хороша. Тем не менее, я думаю, что с двойной проверкой блокировки все в порядке, если вы никогда не забудете пометить управляющую переменную как volatile. Все проблемы с этим методом, которые я видел, предполагают, что переменная не является изменчивой.

...