0

Please see the below static method. I would expect the result of this program to print the values 0-9 in a random order. Instead, there are duplicates in the result (indicating the method SleepAndPrintThreadIndex is not thread-safe). Can you spot the issue?

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

namespace ThreadTest
{
    class Program
    {
        static void Main(string[] args)
        {
            int threadCount = 10;
            List<Task> tasks = new List<Task>();
            for (int i = 0; i < threadCount; i++)
            {
                var task = Task.Factory.StartNew(() => SleepAndPrintThreadIndex(i), TaskCreationOptions.LongRunning);
                tasks.Add(task);
            }
            Task.WaitAll(tasks.ToArray());
            Console.ReadLine();

            // The final result is not 0-9 printed in a random order as expected

            // Instead, the result has duplicate values!  
            // Sample result:

            // 4
            // 6
            // 3
            // 5
            // 4
            // 7
            // 7
            // 9
            // 10
            // 10


        }
        private static void SleepAndPrintThreadIndex(int threadIndex)
        {
            Random r = new Random(DateTime.UtcNow.Millisecond);
            System.Threading.Thread.Sleep(r.Next(5000));
            Console.WriteLine(threadIndex);
        }
    }
}
BlueSky
  • 1,449
  • 1
  • 16
  • 22
  • 2
    Closures capture variables not values. – user4003407 Feb 07 '15 at 03:33
  • Thanks - that sounds like the right answer - but can you please elaborate which line of code you are referring to? – BlueSky Feb 07 '15 at 03:35
  • 1
    @BlueSky the variable `i` is captured in a closure (ie : not copied and passed by value). If you do `int j = i` and call `SleepAndPrintThreadIndex(j)` you'll see that all works as expected, for example. – J... Feb 07 '15 at 03:38
  • This one is a bit more targeted to loops - http://stackoverflow.com/questions/271440/captured-variable-in-a-loop-in-c-sharp - and points out capturing `i` in your case. (did not find as initial duplicate) – Alexei Levenkov Feb 07 '15 at 03:39
  • @BlueSky - Are you aware that calling `new Random(DateTime.UtcNow.Millisecond);` so rapidly means that it is very likely that the call to `r.Next(5000)` will produce the same value for many of the calls to `SleepAndPrintThreadIndex`? – Enigmativity Feb 07 '15 at 04:03
  • @BlueSky - In fact, in a test I ran I failed to get more than one value per run. Not at all random. – Enigmativity Feb 07 '15 at 04:07
  • thanks, yeah, I was aware. I was going to fix it but didn't want to distract from the core question by adding an instance or static variable. appreciate you reaching out though. – BlueSky Feb 07 '15 at 04:12

0 Answers0