Matthew J.
Matthew J.

Reputation: 91

Exception when attempting to modify Dictionary

I was creating a Dictionary to manage Threads and I ran into this exception when I attempt to clear dead threads after adding some Threads into the dictionary.

using System;
using System.Collections.Generic;
using System.Threading;

namespace SysCat {
    public class ThreadManager {
        public static readonly ThreadManager GlobalThreadManager = new ThreadManager( );

        /// <summary>
        /// Initializes a new instance of the <see cref="SysCat.ThreadManager"/> class.
        /// </summary>
        public ThreadManager( ) {

        }

        /// <summary>
        /// Create a new ThreadManager with all the Threads within baseTM and deinitializes baseTM
        /// </summary>
        public ThreadManager( ThreadManager baseTM ) {
            this.threads = baseTM.threads;
            baseTM.threads.Clear( );
        }

        private Dictionary<Guid,Thread> threads = new Dictionary<Guid, Thread>( );

        /// <summary>
        /// Attempts to obtain an unused ThreadID within an attempt limit
        /// </summary>
        /// <returns>The unused ThreadID, if found</returns>
        /// <param name="attempts">The number of iterations to try.</param>
        public Guid getUnusedThreadID( int attempts ) {
            lock( threads ) {
                Guid threadID;
                int totalAttempts = attempts;
                while( threads.ContainsKey( ( threadID = Guid.NewGuid( ) ) ) ) {
                    attempts--;
                    if( attempts == 0 )
                        throw new NoOpenThreadIDException( totalAttempts );
                }
                return threadID;
            }
        }

        /// <summary>
        /// Attempts to get a Thread via its ThreadID, if it exists
        /// </summary>
        /// <returns>The Thread</returns>
        /// <param name="threadID">The ThreadID to use</param>
        public Thread getThread( Guid threadID ) {
            lock( threads ) {
                Thread tryGet;
                if( !threads.TryGetValue( threadID, out tryGet ) )
                    throw new ArgumentException( "Specified ThreadID does not exist in this ThreadManager" );
                return tryGet;
            }
        }

        /// <summary>
        /// Attempts to get a ThreadID via the Thread
        /// </summary>
        /// <returns>The ThreadID</returns>
        /// <param name="thread">The Thread to use</param>
        public Guid getThreadID( Thread thread ) {
            lock( threads ) {
                if( threads.ContainsValue( thread ) ) {
                    foreach( Guid id in threads.Keys ) {
                        Thread t;
                        threads.TryGetValue( id, out t );
                        if( t.Equals( thread ) )
                            return id;
                    }
                    // I should never get here
                    return Guid.Empty;
                }
                else
                    throw new ArgumentException( "Specified Thread does not exist in this ThreadManager" );
            }
        }

        /// <summary>
        /// Adds the Thread, unless it cannot find an open ThreadID
        /// </summary>
        /// <returns>The ThreadID used to register the Thread</returns>
        /// <param name="thread">The Thread to register</param>
        public Guid addThread( Thread thread ) {
            lock( threads ) {
                Guid id;
                try {
                    id = getUnusedThreadID( 500 );
                }
                catch( NoOpenThreadIDException e ) {
                    throw e;
                }
                threads.Add( id, thread );
                return id;
            }
        }

        /// <summary>
        /// Attempts to clear all stopped and null Threads
        /// </summary>
        /// <returns>The number of dead Threads cleared</returns>
        public int clearDeadThreads() {
            int cleared = 0;
            lock(threads) {
                foreach( Guid id in threads.Keys ) {
                    Thread thread;
                    threads.TryGetValue( id, out thread );
                    if( thread == null ) {
                        // Does not exist, Thread is dead
                        cleared++;
                        threads.Remove( id );
                        continue;
                    }
                    if( !thread.IsAlive ) {
                        // Not alive, Thread is dead
                        cleared++;
                        threads.Remove( id );
                        continue;
                    }
                }
            }

            return cleared;
        }

        public class NoOpenThreadIDException : Exception {
            public NoOpenThreadIDException( int attempts )
                : base( string.Format( "Unable to find an open ThreadID within the attempt limit of {0}", attempts ) ) {
            }
        }
    }
}

And I am getting this exception: System.InvalidOperationException with the message: Collection was modified; enumeration operation may not execute

I am unsure why this is happening, any help would be appreciated.

This is the code I used to test it:

using System;
using System.Collections;
using System.IO;
using System.Collections.Generic;
using System.Threading;

using _TM = SysCat.ThreadManager;

namespace SysCat {
    public class SysCat {
        Thread T = new Thread( new ThreadStart( delegate {
            while(true) Thread.Sleep(250);
        } ) );
        Thread T2 = new Thread( new ThreadStart( delegate {

        } ) );
        Thread T3 = new Thread( new ThreadStart( delegate {

        } ) );
        _TM.GlobalThreadManager.addThread( T );
        _TM.GlobalThreadManager.addThread( T2 );
        _TM.GlobalThreadManager.addThread( T3 );
            Console.WriteLine( _TM.GlobalThreadManager.clearDeadThreads( ) );
            Console.ReadLine( );
        }
    }
}

Upvotes: 2

Views: 91

Answers (1)

Hamid Pourjam
Hamid Pourjam

Reputation: 20754

you can not modify an IEnumerable while you are enumerating over it.

you can do a simple hack and get a copy of what you are enumerating so changes in it does not result in System.InvalidOperationException.

for example instead of

foreach( Guid id in threads.Keys) 

use

foreach( Guid id in threads.Keys.ToList()) 

keep in mind that you can use data structures that supports concurrency

Upvotes: 2

Related Questions