CODEWITHSUNDEEP
c#multithreadingcollections
reticent
reticent

Reputation: 1669

InvalidOperationException: Collection was modified - although locking the collection

I have a synchronized hashtable, from which I regularly remove some entries. Multiple threads run this code. So I lock the entire foreach, but I still sometimes get InvalidOperationException: Collection was modified ... at Hashtable.HashtableEnumerator.MoveNext() - i.e. in the foreach loop. What am I doing wrong? Isn't locking enough?

private static readonly Hashtable sessionsTimeoutData = Hashtable.Synchronized(new Hashtable(5000));



private static void ClearTimedoutSessions()
{
    List keysToRemove = new List();
    long now = DateTime.Now.Ticks;
    lock (sessionsTimeoutData)
    {
        TimeoutData timeoutData;
        foreach (DictionaryEntry entry in sessionsTimeoutData)
        {
            timeoutData = (TimeoutData)entry.Value;
            if (now - timeoutData.LastAccessTime > timeoutData.UserTimeoutTicks)
                keysToRemove.Add((ulong)entry.Key);
        }
    }
    foreach (ulong key in keysToRemove)
        sessionsTimeoutData.Remove(key);
}

Upvotes: 7

Views: 2971

Answers (3)

Michael Burr
Michael Burr

Reputation: 340168

You want to lock using SyncRoot which is the object that the methods for a synchronized Hashtable will lock on:

lock (sessionsTimeoutData.SyncRoot)
{
    // ...
}

See http://msdn.microsoft.com/en-us/library/system.collections.hashtable.synchronized.aspx:

Enumerating through a collection is intrinsically not a thread-safe procedure. Even when a collection is synchronized, other threads can still modify the collection, which causes the enumerator to throw an exception. To guarantee thread safety during enumeration, you can either lock the collection during the entire enumeration or catch the exceptions resulting from changes made by other threads.

The following code example shows how to lock the collection using the SyncRoot during the entire enumeration:

Hashtable myCollection = new Hashtable();
lock(myCollection.SyncRoot)
{
    foreach (object item in myCollection)
    {
        // Insert your code here.
    }
}

Upvotes: 8

sbridges
sbridges

Reputation: 25140

You need to lock while you remove as well as while you are calculating what to remove. Move this,

foreach (ulong key in keysToRemove)
        sessionsTimeoutData.Remove(key);

Into your locked section.

Upvotes: 1

Etienne de Martel
Etienne de Martel

Reputation: 36852

Why is the second foreach outside of the lock?

Upvotes: 2

Related Questions