2

I have a simple code which has a lock and creates 5 threads.

static readonly object _object = new object();

static void A(int currentValue)
{
    lock (_object)
    {
        Console.WriteLine(currentValue + " Start");                
        Console.WriteLine(currentValue + " End");
    }
}

static void Main(string[] args)
{
    for (int i = 0; i < 5; i++)
    {
        Thread t = new Thread(() => A(i));
        t.Name = "Thread " + i;
        Console.WriteLine(t.Name + " Created");
        t.Start();                
    }
    Console.ReadKey();
}

When I run this code, the following is displayed in the output.

Thread 0 Created
Thread 1 Created
1 Start
1 End
Thread 2 Created
2 Start
2 End
Thread 3 Created
3 Start
3 End
Thread 4 Created
4 Start
4 End
5 Start
5 End

When I am only creating 5 threads (0 - 4). Why do I see an entry for 5 Start and 5 End?

Kirill Shlenskiy
  • 9,367
  • 27
  • 39
Kaveesh
  • 388
  • 6
  • 14
  • Try pre increment instead of post increment e.g. ++i – Nono Mar 14 '17 at 07:35
  • I'm not sure of the root cause, but I imagine these are related: I notice that you don't have '0 Start' and '0 End' lines. It looks like the i is getting incremented before each thread runs, but is still the same i for the 'thread created' line. My best guess is that when the thread runs, it takes the latest value of i, rather than the value for i at the time you declare the thread variable. Are you consistently getting the same results? (See also Peter Duniho's answer - very relevant) – Jarak Mar 14 '17 at 07:38
  • 3
    The problem is that the variable `i` has changed by the time the thread is started. You are lucky that any thread even gets to start before the entire loop has completed. Normally, when you write a bug like this, the behavior is that _all_ of the threads print `5`. See marked duplicate (Lasse's answer) for details on fixing it. – Peter Duniho Mar 14 '17 at 07:38
  • More importantly, the described behaviour will remain regardless of the `lock` statement being present in the code, so the question is - *why* is there a `lock` statement? – Kirill Shlenskiy Mar 14 '17 at 07:43
  • @KirillShlenskiy It stops two threads calling `Console` methods at the same time and interleaving the two calls (or even interleaving characters written within one call, which I believe can happen). – Rawling Mar 14 '17 at 07:47
  • @Rawling that is true, but then Kaveesh is left with a scrambled for loop output, without the advantage of parallel processing – Mong Zhu Mar 14 '17 at 07:49
  • 1
    @MongZhu I suspect in the real world `A` would be doing some parallelizable work before finally locking in order to write to `Console`, or add output to a collection, or do something else not-thread-safe. – Rawling Mar 14 '17 at 07:53
  • @Rawling I guess you`re right. Just in this context one ends up with a scrambled for-loop – Mong Zhu Mar 14 '17 at 07:55

1 Answers1

0

The problem is that when a thread is started and the call of A happpens the compiler will jump for a brief moment into this line go to get the parameter:

Thread t = new Thread(() => A(i));

if at this point in time the loop is already finished the variable i will have a value of 5. So it will take this value. This phenomenon is called closure I guess. Here is the article The Beauty of Closures by Jon Skeet

You can get out of it if you save the i value into a newly declared temporal variable like this and the 5 will not come upt anymore

for (int i = 0; i < 5; i++)
{
    int temp = i;
    Thread t = new Thread(() => A(temp));
    t.Name = "Thread " + i;
    t.Start();
}

Here is an answer to a very similar problem

The lock is actually not necessary in this context since you access only local variables in the method A. Every thread will have its own personal variable. The lock actually inhibits the threads to run in parallel. Unless you wanted it this way but then you don't need threads.

Community
  • 1
  • 1
Mong Zhu
  • 23,309
  • 10
  • 44
  • 76