BetterLateThanNever
BetterLateThanNever

Reputation: 906

Threading in c# with locks

I think i'm missing some basics in here and unable to fig out the problem..

Output of below program is not as expected. Can some one help me understand the issue here please.

using System;
using System.Threading;

public class Program
{
    private static readonly object _lock = new object();
    public static void Main()
    {
        for (var i = 0; i < 10; i++)
        {
            //Console.WriteLine("Start "+i); 
            System.Threading.Thread thread = new System.Threading.Thread(() => ExecuteInBackground(i));
            thread.IsBackground = true;
            thread.Start();
        }
    }

    private static void ExecuteInBackground(Object obj)
    {
        lock (_lock)
        {
            Console.WriteLine("A "+obj);
            test.ttt(obj);
        }
    }
}


public static class test
{
    public static void ttt(object obj)
    {
        Console.WriteLine("B "+ obj);
    }
}

I'm expecting to see 0 To 9 in output.. But actual output is as follows..

A 1
B 1
A 1
B 1
A 3
B 3
A 4
B 4
A 5
B 5
A 6
B 6
A 7
B 7
A 8
B 8
A 9
B 9
A 10
B 10

Any help is greatly appreciated.

Please feel free to play around with code in https://dotnetfiddle.net/nYfbMU

Thanks, Reddy.

Upvotes: 0

Views: 60

Answers (2)

BetterLateThanNever
BetterLateThanNever

Reputation: 906

This worked for me..

using System;
using System.Threading;

public class Program
{
    private static readonly object _lock = new object();
    public static void Main()
    {
        for (var i = 0; i <= 10; i++)
        {
            fn(i);
        }
        Console.ReadLine();
    }

    private static void fn(int i)
    {
        System.Threading.Thread thread = new System.Threading.Thread(() => ExecuteInBackground(i));
        thread.IsBackground = true;
        thread.Start();
    }

    private static void ExecuteInBackground(Object obj)
    {
        lock (_lock)
        {
            Thread.Sleep(500);
            Console.WriteLine("A "+obj);
            test.ttt(obj);
        }
    }
}
public static class test
{
    //private static readonly object _lock = new object();
    public static void ttt(object obj)
    {
        //lock(_lock)
            Console.WriteLine("B "+ obj);
    }
}

Upvotes: 0

Change this:

for (var i = 0; i < 10; i++)
{
    //Console.WriteLine("Start "+i); 
    System.Threading.Thread thread = new System.Threading.Thread(() => ExecuteInBackground(i));

to this:

for (var i = 0; i < 10; i++)
{
    var temp = i;
    //Console.WriteLine("Start "+i); 
    System.Threading.Thread thread = new System.Threading.Thread(() => ExecuteInBackground(temp));

This is a closure issue. See Why is it bad to use an iteration variable in a lambda expression

The reason the original code doesn't work as you expected, and why the temp variable does, is because () => ExecuteInBackground(i) is like saying "at some point in the future, I want this new thread to call the ExecuteInBackground method, passing in whatever value i has when that call is made". Since the loop variable goes into scope at the start of the loop, and out of scope after the loop is finished, the value of i changes between the time you call Thread, and when ExecuteInBackground executes. By using a temp variable inside the loop, that goes out of scope with every iteration of the loop, each thread's call to ExecuteInBackground is essentially getting a different variable with an unchanging value with each call, and the next incrementing of i doesn't mess things up.

Upvotes: 4

Related Questions