CODEWITHSUNDEEP
c#multithreading
arsen.gandjuns
arsen.gandjuns

Reputation: 15

C# threading: loop counter is printing last value only

Why does this code:

private static void UdpPortListener(UInt16 Port)
{
    Console.WriteLine("Listening to port: {0}", Port);
}

static void Main(string[] args)
{
    List<Thread> ts = new List<Thread>();

    for(int i = 0; i < 20; i++)
    {
        Thread t = new Thread(() =>
        {
            UdpPortListener(Convert.ToUInt16(52000 + i));
        });

        t.IsBackground = false;

        ts.Add(t);
    }

    ts.ForEach((x) => x.Start());
}

produce this output:

Listening to port: 52020
Listening to port: 52020
...
Listening to port: 52020

When I was writing this code I hoped it would print incrementing numbers starting from 52000

Upvotes: 1

Views: 194

Answers (3)

TYY
TYY

Reputation: 2716

The code has 99 problems (just joking) and closure is one of them :). Also you don't need full blown threads you can use tasks.

void static Main()
{
    List<  System.Threading.Tasks.Task> ts = new List<  System.Threading.Tasks.Task>();

    for(int i = 0; i < 20; i++) // why not have i = 52000 ???
    {
        var  num = Convert.ToUInt16(52000 + i);
        var t = new   System.Threading.Tasks.Task(() =>
        {
            UdpPortListener(num);
        });


        ts.Add(t);
    }

    ts.ForEach((x) => x.Start());
}

// Define other methods and classes here

private static void UdpPortListener(UInt16 Port)
{
    Console.WriteLine("Listening to port: {0}", Port);
}

Upvotes: 1

Simon Whitehead
Simon Whitehead

Reputation: 65079

Its the closure you've got there that closes over your for loop variable.

That i variable is promoted at compile time .. because its a loop counter and its actually accessed outside of the loop (in the thread delegate here):

Thread t = new Thread(() =>
{
    UdpPortListener(Convert.ToUInt16(52000 + i));
}); //                                      ^^^ the compiler closes over this

What this means, is that by the time your Threads are spawned up the value of i is checked in your UdpPortListener method ... the value of i is the last value in the for loop.. because the loop executed before it.

To fix this .. you need to copy the value inside loop:

var temp = i;
Thread t = new Thread(() =>
{     
    UdpPortListener(Convert.ToUInt16(52000 + temp));
});

Upvotes: 3

Alex Kopachov
Alex Kopachov

Reputation: 733

This happens due to closure effect. Try this:

static void Main(string[] args)
{
    List<Thread> ts = new List<Thread>();

    for(int i = 0; i < 20; i++)
    {
        var closureIndex = i;
        Thread t = new Thread(() =>
        {
            UdpPortListener(Convert.ToUInt16(52000 + closureIndex));
        });

        t.IsBackground = false;

        ts.Add(t);
    }

    ts.ForEach((x) => x.Start());
}

Upvotes: 2

Related Questions