.netmultithreadingf#thread-safety

Thread-safe enumerator 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?


Solution

  • 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)