Is ReadOnlyCollection threadsafe if the underlying collection is not touched?

Question

MSDN vaguely mentions:

A ReadOnlyCollection<(Of <(T>)>) can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. To guarantee thread safety during enumeration, you can lock the collection during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

Would the following public static collection be safe for multiple threads to iterate over? If not, is there something built into .NET that is safe? Should I just drop the ReadOnlyCollection and create a new copy of a private collection for each access of a SomeStrings property getter? I understand that there could be a deadlock issue if multiple threads tried to lock on the public collection, but this is an internal library, and I can't see why we would ever want to.

public static class WellKnownStrings {

    public static readonly ICollection<string> SomeStrings;

    static WellKnownStrings()
    {
        Collection<string> someStrings = new Collection<string>();
        someStrings.Add("string1");
        someStrings.Add("string2");
        SomeStrings = new ReadOnlyCollection<string>(someStrings);
    }
}

Answer 1

Generally, an immutable object that never changes its internal state (once published to callers outside) can be seen as thread-safe.

A ReadOnlyCollection<T> however is not thread-safe per se, as it is a mere wrapper around an existing collection which its owner could modify any time.

The example in the OP is thread-safe though, because the underlying collection can't be modified (at least not without hacking).

Answer 2

Would you like some strong typing with that?

While your solution was clever, I think this may suit your needs a little better, especially with respect to code reuse.

WellKnownStrings

public class WellKnownStrings : StringEnumeration
{

    private WellKnownStrings(string specialString) :base(specialString)
    {

    }
    public static IEnumerable<String> SpecialStrings
    {
        get
        {
            return GetAllStrings<WellKnownStrings>();
        }
    }

    public static readonly WellKnownStrings String1 = new WellKnownStrings("SOME_STRING_1");
    public static readonly WellKnownStrings String2 = new WellKnownStrings("SOME_STRING_2_SPECIAL");
    public static readonly WellKnownStrings String3 = new WellKnownStrings("SOME_STRING_3_SPECIAL"); 
}

StringEnumeration

This is a base class I've adapted for doing just what you are describing.

public abstract class StringEnumeration : Enumeration
{

    private static int _nextItemValue;
    private static readonly object _initializeLock = new object();


    protected StringEnumeration(string stringValue)
        :base(0, stringValue)
    {
        if(stringValue == null)
        {
            throw new ArgumentNullException("stringValue");
        }
        lock(_initializeLock)
        {
            _nextItemValue++;
            _value = _nextItemValue;
        }
    }

    public static IEnumerable<string> GetAllStrings<T>()
        where T: StringEnumeration
    {
        return GetAll<T>().Select(x => x.DisplayName);
    }

    private readonly int _value;
    public override int  Value
    {
        get 
        {
            return _value;
        }
    }


    public static explicit operator string(WellKnownStrings specialStrings)
    {
        return specialStrings.ToString();
    }


}

The Enumeration base class

Code originally stolen and adapted from Jimmy Bogard's blog The only changes I made was to make the Value property virtual in the derived class, and to make the GetAll() not dependent on a new T() generic parameter, because static member fields do not need an instance to get the value reflectively.

public abstract class Enumeration : IComparable
{
    private readonly int _value;
    private readonly string _displayName;


    protected Enumeration(int value, string displayName)
    {
        _value = value;
        _displayName = displayName;
    }

    public virtual int Value
    {
        get { return _value; }
    }

    public string DisplayName
    {
        get { return _displayName; }
    }

    public override string ToString()
    {
        return DisplayName;
    }

    public static IEnumerable<T> GetAll<T>() where T : Enumeration 
    {
        return typeof(T).GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly)
            .Where(field => field.FieldType == typeof (T))
            .Select(field => field.GetValue(null))
            .Where(value =>value != null)
            .Cast<T>();
    }

    public override bool Equals(object obj)
    {
        var otherValue = obj as Enumeration;

        if (otherValue == null)
        {
            return false;
        }

        var typeMatches = GetType().Equals(obj.GetType());
        var valueMatches = _value.Equals(otherValue.Value);

        return typeMatches && valueMatches;
    }

    public override int GetHashCode()
    {
        return _value.GetHashCode();
    }

    public static int AbsoluteDifference(Enumeration firstValue, Enumeration secondValue)
    {
        var absoluteDifference = Math.Abs(firstValue.Value - secondValue.Value);
        return absoluteDifference;
    }

    public static T FromValue<T>(int value) where T : Enumeration, new()
    {
        var matchingItem = parse<T, int>(value, "value", item => item.Value == value);
        return matchingItem;
    }

    public static T FromDisplayName<T>(string displayName) where T : Enumeration, new()
    {
        var matchingItem = parse<T, string>(displayName, "display name", item => item.DisplayName == displayName);
        return matchingItem;
    }

    private static T parse<T, K>(K value, string description, Func<T, bool> predicate) where T : Enumeration, new()
    {
        var matchingItem = GetAll<T>().FirstOrDefault(predicate);

        if (matchingItem == null)
        {
            var message = string.Format("'{0}' is not a valid {1} in {2}", value, description, typeof(T));
            throw new Exception(message);
        }

        return matchingItem;
    }

    public int CompareTo(object other)
    {
        return Value.CompareTo(((Enumeration)other).Value);
    }
}

    public static IEnumerable<T> GetAll<T>() where T : Enumeration, new()
    {
        var type = typeof(T);
        var fields = type.GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly).Where(field=>);

        foreach (var info in fields)
        {
            var instance = new T();
            var locatedValue = info.GetValue(instance) as T;

            if (locatedValue != null)
            {
                yield return locatedValue;
            }
        }
    }

    public override bool Equals(object obj)
    {
        var otherValue = obj as Enumeration;

        if (otherValue == null)
        {
            return false;
        }

        var typeMatches = GetType().Equals(obj.GetType());
        var valueMatches = _value.Equals(otherValue.Value);

        return typeMatches && valueMatches;
    }

    public override int GetHashCode()
    {
        return _value.GetHashCode();
    }

    public static int AbsoluteDifference(Enumeration firstValue, Enumeration secondValue)
    {
        var absoluteDifference = Math.Abs(firstValue.Value - secondValue.Value);
        return absoluteDifference;
    }

    public static T FromValue<T>(int value) where T : Enumeration, new()
    {
        var matchingItem = parse<T, int>(value, "value", item => item.Value == value);
        return matchingItem;
    }

    public static T FromDisplayName<T>(string displayName) where T : Enumeration, new()
    {
        var matchingItem = parse<T, string>(displayName, "display name", item => item.DisplayName == displayName);
        return matchingItem;
    }

    private static T parse<T, K>(K value, string description, Func<T, bool> predicate) where T : Enumeration, new()
    {
        var matchingItem = GetAll<T>().FirstOrDefault(predicate);

        if (matchingItem == null)
        {
            var message = string.Format("'{0}' is not a valid {1} in {2}", value, description, typeof(T));
            throw new Exception(message);
        }

        return matchingItem;
    }

    public int CompareTo(object other)
    {
        return Value.CompareTo(((Enumeration)other).Value);
    }
}

Also, about the thread safe issue...

The class I provided thread-safe, intuitive, and reusable. Your use case of ReadOnlyCollection<T> was also thread-safe, BUT (as herzmeister der welten) pointed out, this is not the case in many scenarios. It also does not actually expose the writeable ICollection members, because any calls to those throw exceptions.