.net - Thread-safe enumerator is not thread safe - Stack Overflow

I have the following F# types

type ThreadSafeEnumerator<'T>(inner: IEnumerator<'T>, readWriteLock: ReaderWriterLockSlim) =
    do readWriteLock.EnterReadLock()

    interface IEnumerator<'T> with
        member __.Current: 'T = inner.Current
        member __.Current: obj = inner.Current
        member __.Dispose() = 
            inner.Dispose()
            readWriteLock.ExitReadLock()
        member __.MoveNext() = inner.MoveNext()
        member __.Reset() = inner.Reset()

type ThreadSafeObject() = 
    let _lock = new ReaderWriterLockSlim()

    member __.Lock = _lock
    member __.EnterReadLock() = _lock.EnterReadLock()
    member __.EnterWriteLock() = _lock.EnterWriteLock()
    member __.ExitReadLock() = _lock.ExitReadLock()
    member __.ExitWriteLock() = _lock.ExitWriteLock()
    member __.EnterUpgradeableReadLock() = _lock.EnterUpgradeableReadLock()
    member __.ExitUpgradeableReadLock() = _lock.ExitUpgradeableReadLock()

    member this.SafeRead reader = this.EnterReadLock(); try reader() finally this.ExitReadLock()
    member this.SafeWrite writer = this.EnterWriteLock(); try writer() finally this.ExitWriteLock()

    member this.SafeReadWrite readerWriter = 
        this.EnterUpgradeableReadLock(); 
        try readerWriter (fun writer -> this.SafeWrite(writer)) 
        finally this.ExitUpgradeableReadLock()

    member this.Dispose() = 
        _lock.Dispose()
        GC.SuppressFinalize(this)

    override this.Finalize() = this.Dispose()

    interface IDisposable with
        member this.Dispose() = this.Dispose()

type ThreadSafeList<'v>([<Optional; DefaultParameterValue(10)>] initialCapacity: int) =
    inherit ThreadSafeObject()
    let _container = ResizeArray<'v>(initialCapacity)

    member this.Clear() = this.SafeWrite(fun() -> _container.Clear())
    member this.Count = this.SafeRead(fun() -> _container.Count)
    member this.Add(value: 'v) = this.SafeWrite(fun() -> _container.Add(value))
    member this.Contains(value: 'v) = this.SafeRead(fun() -> _container.Contains(value))
    member this.Remove(value: 'v) = this.SafeWrite(fun() -> _container.Remove(value) |> ignore)

    member this.Item
        with get(index: int) = this.SafeRead(fun() -> _container.[index])
        and set(index: int) (value: 'v) = this.SafeWrite(fun() -> _container.[index] <- value)
    
    member this.Trim(index: int) = this.SafeWrite(fun() -> _container.Trim(index))

    member this.Iter(action: 'v -> unit) = this.SafeRead(fun() -> _container |> Seq.iter action)
    member this.Map(action: 'v -> 'r) = this.SafeRead(fun() -> _container |> Seq.map action)

    member private this.GetEnumerator() = new ThreadSafeEnumerator<'v>(_container.GetEnumerator(), this.Lock)

    interface Collections.IEnumerable with
        member this.GetEnumerator() = this.GetEnumerator() :> Collections.IEnumerator

    interface IEnumerable<'v> with
        member this.GetEnumerator() = this.GetEnumerator()

Iterating a ThreadSafeList using the method Iter is thread safe. However, if I iterate using the Enumerator (implicitly through a Seq.iter in F# or a foreach in C#)

    ---- System.InvalidOperationException : Collection was modified; enumeration operation may not execute.

is thrown.

Can someone help me understand why the ThreadSafeEnumerator implementation is not thread safe?

I have the following F# types

type ThreadSafeEnumerator<'T>(inner: IEnumerator<'T>, readWriteLock: ReaderWriterLockSlim) =
    do readWriteLock.EnterReadLock()

    interface IEnumerator<'T> with
        member __.Current: 'T = inner.Current
        member __.Current: obj = inner.Current
        member __.Dispose() = 
            inner.Dispose()
            readWriteLock.ExitReadLock()
        member __.MoveNext() = inner.MoveNext()
        member __.Reset() = inner.Reset()

type ThreadSafeObject() = 
    let _lock = new ReaderWriterLockSlim()

    member __.Lock = _lock
    member __.EnterReadLock() = _lock.EnterReadLock()
    member __.EnterWriteLock() = _lock.EnterWriteLock()
    member __.ExitReadLock() = _lock.ExitReadLock()
    member __.ExitWriteLock() = _lock.ExitWriteLock()
    member __.EnterUpgradeableReadLock() = _lock.EnterUpgradeableReadLock()
    member __.ExitUpgradeableReadLock() = _lock.ExitUpgradeableReadLock()

    member this.SafeRead reader = this.EnterReadLock(); try reader() finally this.ExitReadLock()
    member this.SafeWrite writer = this.EnterWriteLock(); try writer() finally this.ExitWriteLock()

    member this.SafeReadWrite readerWriter = 
        this.EnterUpgradeableReadLock(); 
        try readerWriter (fun writer -> this.SafeWrite(writer)) 
        finally this.ExitUpgradeableReadLock()

    member this.Dispose() = 
        _lock.Dispose()
        GC.SuppressFinalize(this)

    override this.Finalize() = this.Dispose()

    interface IDisposable with
        member this.Dispose() = this.Dispose()

type ThreadSafeList<'v>([<Optional; DefaultParameterValue(10)>] initialCapacity: int) =
    inherit ThreadSafeObject()
    let _container = ResizeArray<'v>(initialCapacity)

    member this.Clear() = this.SafeWrite(fun() -> _container.Clear())
    member this.Count = this.SafeRead(fun() -> _container.Count)
    member this.Add(value: 'v) = this.SafeWrite(fun() -> _container.Add(value))
    member this.Contains(value: 'v) = this.SafeRead(fun() -> _container.Contains(value))
    member this.Remove(value: 'v) = this.SafeWrite(fun() -> _container.Remove(value) |> ignore)

    member this.Item
        with get(index: int) = this.SafeRead(fun() -> _container.[index])
        and set(index: int) (value: 'v) = this.SafeWrite(fun() -> _container.[index] <- value)
    
    member this.Trim(index: int) = this.SafeWrite(fun() -> _container.Trim(index))

    member this.Iter(action: 'v -> unit) = this.SafeRead(fun() -> _container |> Seq.iter action)
    member this.Map(action: 'v -> 'r) = this.SafeRead(fun() -> _container |> Seq.map action)

    member private this.GetEnumerator() = new ThreadSafeEnumerator<'v>(_container.GetEnumerator(), this.Lock)

    interface Collections.IEnumerable with
        member this.GetEnumerator() = this.GetEnumerator() :> Collections.IEnumerator

    interface IEnumerable<'v> with
        member this.GetEnumerator() = this.GetEnumerator()

Iterating a ThreadSafeList using the method Iter is thread safe. However, if I iterate using the Enumerator (implicitly through a Seq.iter in F# or a foreach in C#)

    ---- System.InvalidOperationException : Collection was modified; enumeration operation may not execute.

is thrown.

Can someone help me understand why the ThreadSafeEnumerator implementation is not thread safe?

• .net
• multithreading
• f#
• thread-safety

Share Improve this question Follow edited Jan 11 at 22:30 Michael Liu 55.6k1414 gold badges125125 silver badges156156 bronze badges asked Jan 7 at 1:21 Franco TiveronFranco Tiveron 2,93611 gold badge2828 silver badges4444 bronze badges 7

• 1 Is the underlying collection being modified (by either the same thread or a different thread) while the collection is being enumerated? – Michael Liu Commented Jan 7 at 4:52
• @MichaelLiu No, all operations are performed through the ThreadSafeList interface – Franco Tiveron Commented Jan 7 at 21:26
• 1 Is your code modifying the list through the ThreadSafeList interface (e.g., by calling Add or Remove) while enumerating it? – Michael Liu Commented Jan 7 at 22:25
• 2 .NET collection types generally don't allow you to continue enumerating a collection after it's modified, even when the modification is performed by the same thread. This is a built-in restriction of these collection types, not a problem of thread safety. – Michael Liu Commented Jan 9 at 3:48
• 1 @MichaelLiu That's why the code takes a read lock before starting enumeration, so to prevent any other thread to change it until the enumeration is completed – Franco Tiveron Commented Jan 9 at 21:13

 |  Show 2 more comments

1 Answer 1

Sorted by: Reset to default 2

Take a close look at your GetEnumerator implementation:

member private this.GetEnumerator() = new ThreadSafeEnumerator<'v>(_container.GetEnumerator(), this.Lock)

A thread that calls this method will perform the following steps:

1. Call _container.GetEnumerator().
2. Instantiate a ThreadSafeEnumerator.
3. In the ThreadSafeEnumerator constructor, call EnterReadLock.

After Step 3, the read lock will be held until the enumeration completes and the ThreadSafeEnumerator is disposed, preventing any other thread from modifying the list.

Unfortunately, because Step 1 is executed outside the lock, another thread could sneak in right after it and mutate the list. Then, when the first thread resumes execution, the inner enumerator will throw InvalidOperationException.

To resolve the problem, you must call _container.GetEnumerator() only after obtaining the read lock. One way to do this is to change ThreadSafeEnumerator to accept an IEnumerable instead of an IEnumerator:

type ThreadSafeEnumerator<'T>(enumerable: IEnumerable<'T>, readWriteLock: ReaderWriterLockSlim) =
    do readWriteLock.EnterReadLock()
    let inner = enumerable.GetEnumerator()

Then, in ThreadSafeList's GetEnumerator method, pass _container instead of _container.GetEnumerator:

member private this.GetEnumerator() = new ThreadSafeEnumerator<'v>(_container, this.Lock)