Is getting and setting a simple static properties thread safe?

Question

Possible Duplicate:
Are C# auto-implemented static properties thread-safe?

In the following example class

static class Shared
{
    public static string[] Values { get; set; }
}

many reader threads read the Values string array periodically, while from time to time a single writer will replace the whole array with a new value using the setter. Do I need to use a ReaderWriterLock or is this something C# will handle automatically?

Edit: In my case the only required "thread safety" is: Nothing bad should happen when the writer replaces the array while the reader is searching for a value. I don't care if the readers will use the new value immediately as long as they will use it in the future

Answer 1

This is not thread safe. Yes you will need to use a lock.

Answer 2

It depends on what you mean by thread-safe.

The read and replacement are guaranteed to be atomic, but it's not guaranteed that a read following a write will necessarily read the fresh value.

In response to your edit...

Nothing bad will happen with your existing code (for example, torn reads), but there is no guarantee that your readers will ever see the new value. For example, it's possible -- though perhaps unlikely -- that the old reference would be cached in a register forever.

Answer 3

Without any extra protection, there's no guarantee that a reading thread will ever see the new value. In practice, as long as the reading thread is doing anything significant, it will see the new value. In particular, you'll never see "half" an update, with a reference pointing to dead space.

If you make the field volatile, I believe even this danger is removed - but lock-free programming is generally hard to reason about. This means abandoning it being an automatically-implemented property, of course.

Answer 4

I would lock. For multiple read, occasional write use a ReaderWriterLockSlim - much more efficient than ReaderWriteLock.

static class Shared
{
    private static ReaderWriterLockSlim _rwLock = new ReaderWriterLockSlim();
    private static string[] _values;

    public static string[] Values 
    {
        get
        {
            _rwLock.EnterReadLock();
            try
            {
                return _values;
            }
            finally
            {
                _rwLock.ExitReadLock();
            }
        }
        set
        {
            _rwLock.EnterWriteLock();
            try
            {
                _values = value;
            }
            finally
            {
                _rwLock.ExitWriteLock();
            }
        }
    }
}

Answer 5

This use is thread safe:

string[] localValues = Shared.Values;
for (int index = 0; index < localValues.length; index++)
    ProcessValues(localValues[index]);

This use is not thread safe, and can result in out-of-bounds exceptions:

for (int index = 0; index < Shared.Values.Length; index++)
    ProcessValues(Shared.Values[index]);

I would prefer to make thread safe calls more natural by doing something like this:

static class Shared   
{
    private static string[] values;   
    public static string[] GetValues() { return values; }
    public static void SetValues(string[] values) { Shared.values = values; }
}

Of course, the users can still put GetValues() in the loops, and it would be just as bad, but at least it's obviously bad.

Depending on the situation, an even better solution might be to hand out copies, so that the calling code can't mutate the array at all. This is what I would usually do, but it may not be appropriate in your situation.

static class Shared
{
    private static string[] values;
    public static string[] GetValues()
    {
        string[] currentValues = values;
        if (currentValues != null)
            return (string[])currentValues.Clone();
        else
            return null;
    }
    public static void SetValues(string[] values)
    {
        Shared.values = values;
    }
}

Answer 6

To go slightly off-topic, the Java 2 1.5 memory model adds two guarantees (see Java theory and practice: Fixing the Java Memory Model, Part 2):

• volatile reads/writes are also memory barriers.
• final fields appear completely initialized outside of the constructor.

The latter is an interesting case: you used to be able to do foo = new String(new char[]{'a','b','c'}); in one thread and foo = "123"; System.out.println(foo) in another thread and print the empty string, because there was no guarantee that the writes to foo's final fields will happen before the write to foo.

I'm not sure of the details of .NET array initialization; it might be the case that you need to use Thread.MemoryBarrier() (at the start of the getter and at the end of the setter) to ensure that readers only see completely initialized arrays.