Synchronized IEnumerator<T>

Posted by Dan Bryant on Stack Overflow See other posts from Stack Overflow or by Dan Bryant
Published on 2010-04-11T14:49:02Z Indexed on 2010/04/11 16:03 UTC
Read the original article Hit count: 465

Filed under:
|
|
|

I'm putting together a custom SynchronizedCollection<T> class so that I can have a synchronized Observable collection for my WPF application. The synchronization is provided via a ReaderWriterLockSlim, which, for the most part, has been easy to apply. The case I'm having trouble with is how to provide thread-safe enumeration of the collection. I've created a custom IEnumerator<T> nested class that looks like this:

    private class SynchronizedEnumerator : IEnumerator<T>
    {
        private SynchronizedCollection<T> _collection;
        private int _currentIndex;

        internal SynchronizedEnumerator(SynchronizedCollection<T> collection)
        {
            _collection = collection;
            _collection._lock.EnterReadLock();
            _currentIndex = -1;
        }

        #region IEnumerator<T> Members

        public T Current { get; private set;}

        #endregion

        #region IDisposable Members

        public void Dispose()
        {
            var collection = _collection;
            if (collection != null)
                collection._lock.ExitReadLock();

            _collection = null;
        }

        #endregion

        #region IEnumerator Members

        object System.Collections.IEnumerator.Current
        {
            get { return Current; }
        }

        public bool MoveNext()
        {
            var collection = _collection;
            if (collection == null)
                throw new ObjectDisposedException("SynchronizedEnumerator");

            _currentIndex++;
            if (_currentIndex >= collection.Count)
            {
                Current = default(T);
                return false;
            }

            Current = collection[_currentIndex];
            return true;
        }

        public void Reset()
        {
            if (_collection == null)
                throw new ObjectDisposedException("SynchronizedEnumerator");

            _currentIndex = -1;
            Current = default(T);
        }

        #endregion
    }

My concern, however, is that if the Enumerator is not Disposed, the lock will never be released. In most use cases, this is not a problem, as foreach should properly call Dispose. It could be a problem, however, if a consumer retrieves an explicit Enumerator instance. Is my only option to document the class with a caveat implementer reminding the consumer to call Dispose if using the Enumerator explicitly or is there a way to safely release the lock during finalization? I'm thinking not, since the finalizer doesn't even run on the same thread, but I was curious if there other ways to improve this.


EDIT

After thinking about this a bit and reading the responses (particular thanks to Hans), I've decided this is definitely a bad idea. The biggest issue actually isn't forgetting to Dispose, but rather a leisurely consumer creating deadlock while enumerating. I now only read-lock long enough to get a copy and return the enumerator for the copy.

© Stack Overflow or respective owner

Related posts about thread

Related posts about synchronization