Question

Let's say I want to create a collection class that is thread-safe by default.

Internally, the class has a protected List<T> property called Values.

For starters, it makes sense to have the class implement ICollection<T>. Some of this interface's members are quite easy to implement; for example, Count returns this.Values.Count.

But implementing ICollection<T> requires me to implement IEnumerable<T> as well as IEnumerable (non-generic), which is a bit tricky for a thread-safe collection.

Of course, I could always throw a NotSupportedException on IEnumerable<T>.GetEnumerator and IEnumerable.GetEnumerator, but that feels like a cop-out to me.

I already have a thread-safe getValues function which locks on Values and returns a copy in the form of a T[] array. So my idea was to implement GetEnumerator by returning this.getValues().GetEnumerator() so that the following code would actually be thread-safe:

ThreadSafeCollection coll = new ThreadSafeCollection ();

// add some items to coll

foreach (T value in coll) {
    // do something with value
}

Unfortunately this implementation only seems to work for IEnumerable.GetEnumerator, not the generic version (and so the above code throws an InvalidCastException).

One idea I had, which seemed to work, was to cast the T[] return value from getValues to an IEnumerable<T> before calling GetEnumerator on it. An alternative would be to change getValues to return an IEnumerable<T> in the first place, but then for the non-generic IEnumerable.GetEnumerator simply cast the return value from getValues to a non-generic IEnumerable. But I can't really decide if these approaches feel sloppy or perfectly acceptable.

In any case, does anyone have a better idea of how to go about doing this? I have heard of the .Synchronized methods, but they seem to only be available for non-generic collections in the System.Collections namespace. Maybe there is a generic variant of this that already exists in .NET that I simply don't know about?

Was it helpful?

Solution

Most collections specify that you cannot add or remove things from the collection while iterating. It follows that for a thread-safe collection, you want to lock out other threads from modifying the collection while any thread is iterating. This should be easy to do with the iterators syntax, and doesn't require you to make a copy:

public IEnumerator<T> GetEnumerator()
{
    lock (this.Values) // or an internal mutex you use for synchronization
    {
        foreach (T val in this.Values)
        {
            yield return val;
        }
    }
    yield break;
}

This assumes that all of the other operations that might modify the collection also lock this.Values. As long as that's true, you should be fine.

OTHER TIPS

Unfortunately this implementation only seems to work for IEnumerable.GetEnumerator, not the generic version (and so the above code throws an InvalidCastException).

Seems strange for me. Have you implemented non-generic IEnumerable explicitly? I.e. have you written

public IEnumerator<T> GetEnumerator() { ...}
IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator<T>(); }

Also, have you tried to implement IEnumerable with iterators syntax. It should be easy:

public IEnumerator<T> GetEnumerator()
{
    T[] values;
    lock(this.Values)
        values = this.Values.ToArray();
    foreach(var value in values)
        yield return value;
}

If you've tried, why doesn't it suits your needs?

The type T[] has a special realtionship to the type System.Array from which it derives. Arrays like T[] were made before generics were introduced in .NET (otherwise the syntax could have been Array<T>).

The type System.Array has one GetEnumerator() instance method, public. This method returns non-generic IEnumerator (since .NET 1). And System.Array has no explicit interface implementations for GetEnumerator(). This explains your observations.

But when generics were introduced in .NET 2.0, a special "hack" was made to have a T[] implement IList<T> and its base interfaces (of which IEnumerable<T> is one). For this reason I think it's perfectly reasonable to use:

((IList<T>)(this.getValues())).GetEnumerator()

In the version of .NET where I'm checking this, the following classes exist:

namespace System
{
  public abstract class Array
  {
    private sealed class SZArrayEnumerator  // instance of this is returned with standard GetEnumerator() on a T[]
    {
    }
  }

  internal sealed class SZArrayHelper
  {
    private sealed class SZGenericArrayEnumerator<T>  // instance of this is returned with generic GetEnumerator() on a T[] which has been cast to IList<T>
    {
    }
  }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top