Thread-safe enumeration of shared memory that can be updated or deleted

Question

I have a shared object between threads that is used to hold file state information. The object that holds the information is this class:

/// <summary>
/// A synchronized dictionary class.
/// Uses ReaderWriterLockSlim to handle locking. The dictionary does not allow recursion by enumeration. It is purly used for quick read access.
/// </summary>
/// <typeparam name="T">Type that is going to be kept.</typeparam>
public sealed class SynchronizedDictionary<U,T> : IEnumerable<T>
{
    private System.Threading.ReaderWriterLockSlim _lock = new System.Threading.ReaderWriterLockSlim();
    private Dictionary<U, T> _collection = null;

    public SynchronizedDictionary()
    {
        _collection = new Dictionary<U, T>();
    }

    /// <summary>
    /// if getting:
    /// Enters read lock.
    /// Tries to get the value.
    /// 
    /// if setting:
    /// Enters write lock.
    /// Tries to set value.
    /// </summary>
    /// <param name="key">The key to fetch the value with.</param>
    /// <returns>Object of T</returns>
    public T this[U key]
    { 
        get
        {
            _lock.EnterReadLock();
            try
            {
                return _collection[key];
            }
            finally
            {
                _lock.ExitReadLock();
            }
        }

        set
        {
            Add(key, value);
        }

    }

    /// <summary>
    /// Enters write lock. 
    /// Removes key from collection
    /// </summary>
    /// <param name="key">Key to remove.</param>
    public void Remove(U key)
    {
        _lock.EnterWriteLock();
        try
        {
            _collection.Remove(key);
        }
        finally
        {
            _lock.ExitWriteLock();
        }
    }

    /// <summary>
    /// Enters write lock.
    /// Adds value to the collection if key does not exists.
    /// </summary>
    /// <param name="key">Key to add.</param>
    /// <param name="value">Value to add.</param>
    private void Add(U key, T value)
    {
        _lock.EnterWriteLock();
        if (!_collection.ContainsKey(key))
        {
            try
            {
                _collection[key] = value;
            }
            finally
            {
                _lock.ExitWriteLock();
            }
        }

    }

    /// <summary>
    /// Collection does not support iteration.
    /// </summary>
    /// <returns>Throw NotSupportedException</returns>
    public IEnumerator<T> GetEnumerator()
    {
        throw new NotSupportedException();
    }

    /// <summary>
    /// Collection does not support iteration.
    /// </summary>
    /// <returns>Throw NotSupportedException</returns>
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        throw new NotSupportedException();
    }

}

I call this dictionary like this: SynchronizedDictionary _cache = new SynchronizedDictionary();

Other threads can be spawned and use the thread like this: _cache["key"];

The dictionary can be modified at runtime. I see no problem here. Or am I wrong? The problem, in my eyes, lies in the enumerator, because I want to make an enumerator that iterates over the collection. How do I do this? I have thought of these three solutions:

1. Making a Enumerator like this: http://www.codeproject.com/Articles/56575/Thread-safe-enumeration-in-C (but using ReaderWriterLockSlim)
2. Expose the lock object, like SyncRoot does (but with ReaderWriterLockSlim), so a caller calls the enter and exit read methods.
3. Use a database (SQLite fx) instead, holding the information.

The problem with number 1) is:

1. it uses the contructor to entry read mode. What if the GetEnumerator() is call manually, not using the foreach? And forget calling dispose.
2. I do not know if this is a good coding style. Even though I like the code.
3. If the caller uses a foreach, I do not know what the caller might do between the instantiation of the enumerator and the call to dispose. If I have understood the documentation I have read correct this can end up blocking the writer as long as there is one reader left doing some heavy work.

The problem with number 2) is:

1. I do not like exposing this. I know that the .NET API does it, but do not like it.
2. It is up to the caller to enter and exit properly

There is no problem with 3) I my eyes. But I am doing this small project as a spare time project and I want to learn more about multi-threading and reflection, so I want to keep this as a last option. The reason why I want to iterate over the collection at runtime is that I want to find the values, that matches some criteria.

Maybe it is just me that have invented a problem?

I know of ConcurrentDictionary, but I do not want to use this. I am using this project as a playground. Playing with threading and reflection.

EDIT

I have been asked what it is that I am reading and writing. And I am going to tell this in this edit. I am reading and writing this class:

public class AssemblyInformation
{
    public string FilePath { get; private set; }
    public string Name { get; private set; }

    public AssemblyInformation(string filePath, string name)
    {
        FilePath = filePath;
        Name = name;
    }
}

I am doing alot of reads, and almost no writes at runtime. Maybe I will do 2000 and 1 write. There is not going to be alot of object either, maybe 200.

Answer 1

I'll treat your questions as a request for feedback which helps you learn. Let me address the three solutions you have already identified:

1. Yes, this is why such a design should never be exposed as an API to a 3rd-party (or even other developers). It is tricky to use correctly. This codeproject article has some nasty advice.
2. Much better because this model would be explicit about locking, not implicit. However this violates separation of concerns in my opinion.
3. Not sure what you mean here. You could have a Snapshot() method on your dictionary which does a read-only copy which can be safely passed around and read. This is a different trade-off than solution 1.

There is a different solution entirely: Use an immutable dictionary. Such a dictionary could be passed around, read and enumerated safely even under concurrent write access. Such dictionaries/maps are commonly implemented using trees.

I'll elaborate more on a key point: You need to think about the concurrent system as a whole. You cannot make you app correct by making all components thread-safe (in your case a dictionary). You need to define, what you are using the dictionary for.

You say:

The reason why I want to iterate over the collection at runtime is that I want to find the values, that matches some criteria.

You you have concurrent writes happening to the data and want to get a consistent snapshot atomically from the dictionary (maybe to shot some progress report in the UI?). Now that we know this goal, we can devise a solution:

You could add a Clone method to your dictionary which clones all data while taking the read-lock. This will give the caller a fresh object which it can then enumerate over independently. This would be a clean and safely exposable API.

Answer 2

Instead of implementing IEnumerable directly I would add a Values property (like Dictionary.Values):

public IEnumerable<T> Values {
  get {
    _lock.EnterReadLock();
    try {
      foreach (T v in _collection.Values) {   
        yield return v;
      }
    } finally {
      _lock.ExitReadLock();
    }
  }
}