MultiThreading Example - Do I need to Lock Dictionary

Question

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.

You aren't accessing the `Dictionary` inside the thread, so I don't think you need to worry about locking access to it (unless of course your second code block is running multithreaded). — M.Babcock, Apr 18 '12 at 12:41
i think answers u'll get would be too much of a developer's personal preference. If your program works and you want to improve specific part of the code, should highlight that part but to ask a general question does it look good.. — Churk, Apr 18 '12 at 12:45
@Churk - There is absolutely nothing wrong with this question, and thread-safety is not a matter of personal preference. — M.Babcock, Apr 18 '12 at 12:46
@M.Babcock, Looking at the code, there is no real reason to discuss thread safe here, it simple arithmetic, only shared resource is the collection, and all you need to do to make a thread safe collection is wrap it and put synchonize on it: http://msdn.microsoft.com/en-us/library/573ths2x(v=vs.71).aspx — Churk, Apr 18 '12 at 12:51
@ek_ny if you want to see how to make dictionary thread safe, here is a past post: http://stackoverflow.com/questions/157933/whats-the-best-way-of-implementing-a-thread-safe-dictionary-in-net — Churk, Apr 18 '12 at 12:54

score 4 · Accepted Answer · answered Apr 18 '12 at 12:42

4

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.

answered Apr 18 '12 at 12:42

Daniel Hilgarth

171,043
40
335
443

Daniel, thanks. So if I grab my Dictionary and access a LongProcess in order to find out it's status... do I have to worry about the LongProcess getting updated by another Thread? – ek_ny Apr 18 '12 at 13:00
@ek_ny: This really depends on where this happens. If there always is only one process at a time accessing the dictionary, you don't need to worry. However, if you have a little website with a button that lets you start a new LongProcess, you should either lock the dictionary or use the concurrent dictionary, because being a website multiple users could click that button at the same time. – Daniel Hilgarth Apr 18 '12 at 13:05
How about in an action method of a controller in an MVC application. public JsonResult Status() { return new JsonResult { Data = LongProcess.Monitor.Values, JsonRequestBehavior = JsonRequestBehavior.AllowGet }; } – ek_ny Apr 18 '12 at 13:08
I think that could be problematic, but I am not sure. I don't know if there will be multiple threads in an MVC application, one for each request by a user. – Daniel Hilgarth Apr 18 '12 at 13:11

score 2 · Answer 2 · answered Apr 18 '12 at 13:20

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.

score 0 · Answer 3 · answered Apr 18 '12 at 12:43

0

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

answered Apr 18 '12 at 12:43

phlaz

19
1
2

And there are easily searchable versions for 3.5 – asawyer Apr 18 '12 at 12:44
-1: Not correct. In his code, the dictionary is not being accessed by multiple threads. – Daniel Hilgarth Apr 18 '12 at 12:44

MultiThreading Example - Do I need to Lock Dictionary

3 Answers3