Reputation: 10919
Simple copy pasta here:
static void Main(string[] args)
{
List<Task> Tasks = new List<Task>();
Random r = new Random();
for (int o = 0; o < 5; o++)
Tasks.Add(Task.Factory.StartNew(() => { int i = r.Next(0, 3000); Thread.Sleep(i); Console.WriteLine("{0}: {1}", o, i); }));
Task.WaitAll(Tasks.ToArray());
Console.Read();
}
When you run that, you will get something like this:
5: 98
5: 198
5: 658
5: 1149
5: 1300
What am I not understanding about this? Writing each iteration of o
is showing as 5 for all threads when I'd expect to see numbers 0 through 4 in random order.
I tried using an actual method instead of anonymous and it does the same thing. What am I missing?
Edit: I just found the problem with my very first post and edited the question, so sorry if you answered about the improper order problem. However, I am curious as to why o
is not writing properly.
Upvotes: 0
Views: 65
Reputation: 2403
I think you are making the assumption that the tasks are executed in the order they are created, and the TPL makes no such guarantees...
As for the 'o' parameter always printing as 5, that is because it is a local variable in the parent scope of the anonymous function, hence when the print is actually executed its value is 5 because the loop has completed (compare to 'i' being scoped within the anonymous function)
Upvotes: 1
Reputation: 24167
There are at least two problems here.
The problem with o
having the value of 5 on every iteration is one of those "gotchas" of lexical closures. If you want o
to capture its current value, you must create a locally scoped variable inside your loop and use that in your lambda, e.g.:
for (int o = 0; o < 5; ++o)
{
int localO = o;
// now use "localO" in your lambda ...
}
Also, Random
is not thread-safe. Using the same instance of Random
simultaneously across multiple threads can corrupt its state and give you unexpected results.
Upvotes: 2
Reputation: 160882
() =>
{
int i = r.Next(0, 3000);
Thread.Sleep(i);
Console.WriteLine("{0}: {1}", o, i);
})
Your are closing over your loop variable o
with the delegate you use for your task - by the time it is executed your loop has finished and you only get the end value 5 for o
. Remember you are creating a closure over the loop variable, not it's current value - the value is only evaluated when the delegate is executed once the task is started.
You have to create a local copy of the loop variable instead, which you can then use safely:
for (int o = 0; o < 5; o++)
{
int localO = o;
Tasks.Add(Task.Factory.StartNew(() => { int i = r.Next(0, 3000); Thread.Sleep(i); Console.WriteLine("{0}: {1}", localO, i); }));
}
Upvotes: 3