CODEWITHSUNDEEP
c#multithreadinglocking
ek_ny
ek_ny

Reputation: 10243

MultiThreading Example - Do I need to Lock Dictionary

I was asked by one of my students to create an example which would explain multi-threading. I came up with this pseudo example which will spawn threads which will loop a random number of times and sleep while looping. I am keeping track of these threads via a Dictionary which uses a Guid to identify each thread.

The Dictionary could be used on pages used to monitor these threads and possibly "kill" them.

Does this look reasonable:

public class LongProcess
    {
        public static Dictionary<Guid, LongProcess> Monitor = new Dictionary<Guid, LongProcess>();

        public Guid Id { get; set; }

        public int Iterations { get; set; }//number of loops

        public int SleepFactor { get; set; }//rest period while looping

        public int CompletedIterations { get; set; }//number of completed loops


        void Process()
        {
            for(var i = 0 ; i < Iterations; i++)
            {
                Thread.Sleep(1000*SleepFactor);
                SleepFactor = new Random(DateTime.Now.Millisecond).Next(1, 5);
                this.CompletedIterations++;
            }
        }

        public void Start()
        {
            Monitor.Add(Id, this);
            var thread = new Thread(new ThreadStart(Process));
            thread.Start();
        }
    }

Here is how this class would be used:

    var id = Guid.NewGuid();
    var rnd = new Random(DateTime.Now.Millisecond);
    var process = new LongProcess
                      {
                          Iterations = rnd.Next(5, 20),
                          SleepFactor = rnd.Next(1, 5),
                          Id = id
                      };
    process.Start();

any ideas would be appreciated.

Upvotes: 1

Views: 441

Answers (3)

Fen
Fen

Reputation: 943

Other people have answered about the lock around the dictionary and I would agree with them.

The one thing I would change is the setters on the properties. They are currently public which means you can modify the underlying value while the thread is iterating which is not a great api design.

I would add a constructor to the class to take the parameters you want and make the setters private so nobody can accidentally change the values on your class leading to weird results.

Upvotes: 2

phlaz
phlaz

Reputation: 19

You will need to lock the dictionary is it will be accessed by multiple threads or you could use a System.Collections.Concurrent.ConcurrentDictionary which is thread-safe by design

Upvotes: 0

Daniel Hilgarth
Daniel Hilgarth

Reputation: 174457

As long as all calls to process.Start occur on the same thread, you don't need to lock the dictionary, because it isn't accessed in the background threads that are started inside the LongProcess class. That means: Your class LongProcess is not thread safe, but in your current code this isn't a problem.
To be on the safe side and to avoid locking, you can use the new ConcurrentDictionary<TKey, TValue> from .NET4.

Upvotes: 4

Related Questions