Read the updated value from the Interlocked variable, and only one entry in the variable

Question

I would like to create a class with two methods:

• void SetValue(T value) stores the value, but allows only one value to be stored (otherwise it throws an exception).
• T GetValue() retrieves the value (and throws an exception if there is no value yet).

I have the following desires / limitations:

• Reading the value should be cheap.
• Writing a value can be (moderately) expensive.
• GetValue() should throw an exception only if the last value is absent ( null ): it should not throw an exception based on the deprecated null value after calling SetValue() on another thread.
• The value is recorded only once. This means that GetValue() does not need to update the value if it is not null.
• If you can avoid the complete memory barrier, then it is (much) better.
• I get that lock-free concurrency is better, but I'm not sure if it is.

I came up with several ways to achieve this, but I'm not sure which ones are correct, effective, why they are correct and effective, and if there is a better way to achieve what I want.

Method 1

• Using a non-volatile field
• Using Interlocked.CompareExchange to write to a field
• Using Interlocked.CompareExchange to read from a field
• This depends on the (possibly incorrect) assumption that after executing Interlocked.CompareExchange(ref v, null, null) , the next access will be obtained in the field, receiving values ​​that are at least as recent as those that are Interlocked.CompareExchange seen.

The code:

 public class SetOnce1<T> where T : class { private T _value = null; public T GetValue() { if (_value == null) { // Maybe we got a stale value (from the cache or compiler optimization). // Read an up-to-date value of that variable Interlocked.CompareExchange<T>(ref _value, null, null); // _value contains up-to-date data, because of the Interlocked.CompareExchange call above. if (_value == null) { throw new System.Exception("Value not yet present."); } } // _value contains up-to-date data here too. return _value; } public T SetValue(T newValue) { if (newValue == null) { throw new System.ArgumentNullException(); } if (Interlocked.CompareExchange<T>(ref _value, newValue, null) != null) { throw new System.Exception("Value already present."); } return newValue; } } 

Method 2

• Using the volatile field
• Using locknterlocked.CompareExchange to write the value (with [Joe Duffy](http://www.bluebytesoftware.com/blog/PermaLink,guid,c36d1633-50ab-4462-993e-f1902f8938cc.aspx)'s #pragma to avoid the compiler warning on passing a volatile value by ref`).
• Reading the value directly, since the field is volatile

The code:

 public class SetOnce2<T> where T : class { private volatile T _value = null; public T GetValue() { if (_value == null) { throw new System.Exception("Value not yet present."); } return _value; } public T SetValue(T newValue) { if (newValue == null) { throw new System.ArgumentNullException(); } #pragma warning disable 0420 T oldValue = Interlocked.CompareExchange<T>(ref _value, newValue, null); #pragma warning restore 0420 if (oldValue != null) { throw new System.Exception("Value already present."); } return newValue; } } 

Method 3

• Use of non-volatile field.
• Use recording while recording.
• Using lock when reading, if we read null (we get a coherent value outside the lock, since links are read atomically).

The code:

 public class SetOnce3<T> where T : class { private T _value = null; public T GetValue() { if (_value == null) { // Maybe we got a stale value (from the cache or compiler optimization). lock (this) { // Read an up-to-date value of that variable if (_value == null) { throw new System.Exception("Value not yet present."); } return _value; } } return _value; } public T SetValue(T newValue) { lock (this) { if (newValue == null) { throw new System.ArgumentNullException(); } if (_value != null) { throw new System.Exception("Value already present."); } _value = newValue; return newValue; } } } 

Method 4

• Using a volatile field
• Write value using lock.
• Reading the value directly, since the field is volatile (we will get a coherent value, even if we do not use locking, since links are read atomically).

The code:

 public class SetOnce4<T> where T : class { private volatile T _value = null; public T GetValue() { if (_value == null) { throw new System.Exception("Value not yet present."); } return _value; } public T SetValue(T newValue) { lock (this) { if (newValue == null) { throw new System.ArgumentNullException(); } if (_value != null) { throw new System.Exception("Value already present."); } _value = newValue; return newValue; } } } 

Other methods

I could also use Thread.VolatileRead() to read the value in combination with any of the writing methods.

Answer 1

Well, I'm not sure about volatility, but if you don't mind a little abuse and call the second method ... (also it does not depend on nullability, it is freely used for value types) Avoids null checks in getter too. Only locking is performed when writing, so AFAIK, the only negative effect occurs when a delegate is called when a value is received.

 public class SetOnce<T> { private static readonly Func<T> NoValueSetError = () => { throw new Exception("Value not yet present.");}; private Func<T> ValueGetter = NoValueSetError; private readonly object SetterLock = new object(); public T SetValue(T newValue) { lock (SetterLock) { if (ValueGetter != NoValueSetError) throw new Exception("Value already present."); else ValueGetter = () => newValue; } return newValue; } public T GetValue() { return ValueGetter(); } } 

Actually, I feel really stupid about it and feel a little offensive. I would be interested to see comments about possible issues with this. :)

EDIT: just realized that this means that the first call to SetValue(null) means that "null" will be considered a valid value and will return null without exception. Not sure if this is what you would like (I don’t understand why null cannot be a valid value, but if you want to avoid this, just do the check in the installer, there is no need for getter)

EDITx2: if you still want to limit it to class and avoid null values, a simple change could be:

 public class SetOnce<T> where T : class { private static readonly Func<T> NoValueSetError = () => { throw new Exception("Value not yet present.");}; private Func<T> ValueGetter = NoValueSetError; private readonly object SetterLock = new object(); public T SetValue(T newValue) { if (newValue == null) throw new ArgumentNullException("newValue"); lock (SetterLock) { if (ValueGetter != NoValueSetError) throw new Exception("Value already present."); else ValueGetter = () => newValue; } return newValue; } public T GetValue() { return ValueGetter(); } } 

Answer 2

None of your methods except one with lock (# 3) will work correctly.

Appearance:

  if (_value == null) { throw new System.Exception("Value not yet present."); } return _value; 

this code is not atomic, not thread safe unless inside the lock . It is still possible that other threads overlap from _value to null between if and return . You can set a local variable:

  var localValue = _value; if (localValue == null) { throw new System.Exception("Value not yet present."); } return localValue; 

But still, it can return the value of the stall. It is better to use lock - clear, easy and fast.

Change Avoid using lock(this) , because this displayed externally, and third-party code can select lock for your object.

Edit 2: if null can never be set, just do:

 public T GetValue() { if (_value == null) { throw new System.Exception("Value not yet present."); } return _value; } public T SetValue(T newValue) { lock (writeLock) { if (newValue == null) { throw new System.ArgumentNullException(); } _value = newValue; return newValue; } }