Spot the Bug: Fun Concurrency Bug

[Chris Mullins [MVP - C#]]

I hit this bug last night in some test code I had written. It took a few
minutes to figure out what the root cause was, and it left me thinking,
"Wow. That was an interesting one that doesn't come up very often!".

So, for fun, who sees the bug and can explain why it's happening?

List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
Thread t = new Thread(delegate ()
{
Debug.Assert(i != 2);
});

t.Start();
threads.Add(t);
}

foreach (Thread thread in threads)
thread.Join();

--
Chris Mullins

[Kalpesh]

Jon,

I do not understand much of threading & was curious
So, I ran this using snippet compiler & couldn't see any difference in
output

<code>
using System;
using System.Collecti ons.Generic;
using System.Diagnost ics;
using System.Threadin g;

class Sample
{
public static void Main(string[] args)
{
List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
int j = i;
Thread t = new Thread(delegate ()
{
Console.WriteLi ne("inside thread: " + j);
Debug.Assert(j != 2);
});

Console.WriteLi ne("starting thread: " + i);
t.Start();
threads.Add(t);

}

foreach (Thread thread in threads)
thread.Join();
}
}
</code>

Could you explain, what is the problem with having to introduce j as a
temp. variable?
What is the problem with the original code (without introduction of
temp. variable inside the loop)?

Thanks
Kalpesh
On Oct 24, 12:28 pm, Jon Skeet [C# MVP] <[e-mail] mwrote:

Chris Mullins [MVP - C#] <cmull...@yahoo .comwrote:

I hit this bug last night in some test code I had written. It took a few
minutes to figure out what the root cause was, and it left me thinking,
"Wow. That was an interesting one that doesn't come up very often!".

So, for fun, who sees the bug and can explain why it's happening?

List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
Thread t = new Thread(delegate ()
{
Debug.Assert(i != 2);
});

t.Start();
threads.Add(t);
}

foreach (Thread thread in threads)
thread.Join();

Posting without reading any responses... "i" is captured and only has a
single "instance", so the assertion will fail if the thread executes
after the loop has "finished".

The solution:

List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
int j=i; // THIS IS THE CHANGE
Thread t = new Thread(delegate ()
{
Debug.Assert(j != 2); // AND THIS
});

t.Start();
threads.Add(t);

}

foreach (Thread thread in threads)
thread.Join();

There's a new instance of "j" each time we go round the loop.

--
Jon Skeet - <[e-mail] m>http://www.pobox.com/~skeet Blog:http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Willy Denoyette [MVP]]

"Jon Skeet [C# MVP]" <sk***@pobox.co mwrote in message
news:MP******** *************@m snews.microsoft .com...

Chris Mullins [MVP - C#] <cm******@yahoo .comwrote:

>I hit this bug last night in some test code I had written. It took a few
minutes to figure out what the root cause was, and it left me thinking,
"Wow. That was an interesting one that doesn't come up very often!".

So, for fun, who sees the bug and can explain why it's happening?

List<Threadthr eads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
Thread t = new Thread(delegate ()
{
Debug.Assert(i != 2);
});

t.Start();
threads.Add(t);
}

foreach (Thread thread in threads)
thread.Join();

Posting without reading any responses... "i" is captured and only has a
single "instance", so the assertion will fail if the thread executes
after the loop has "finished".

The solution:

List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
int j=i; // THIS IS THE CHANGE
Thread t = new Thread(delegate ()
{
Debug.Assert(j != 2); // AND THIS
});

t.Start();
threads.Add(t);
}

foreach (Thread thread in threads)
thread.Join();

There's a new instance of "j" each time we go round the loop.

Or:
for (int i = 0; i < 2; i++)
{
Thread t = new Thread((Paramet erizedThreadSta rt)delegate(obj ect
y){ Debug.Assert((i nt)y != 2); });
t.Start(i);
threads.Add(t);
}

Or in C# V3 :
for (int i = 0; i < 2; i++)
{
Thread t = new Thread(y = { Debug.Assert((i nt)y != 2); });
t.Start(i);
threads.Add(t);
}
Willy.

[Jon Skeet [C# MVP]]

Kalpesh <sh*********@gm ail.comwrote:

<snip>

Could you explain, what is the problem with having to introduce j as a
temp. variable?
What is the problem with the original code (without introduction of
temp. variable inside the loop)?

It's not really a concurrency issue - it's an anonymous method issue.
The two new threads (and the continuing method) are all still using the
same variable.

See http://pobox.com/~skeet/csharp/csharp2/delegates.html for more -
look at the last section (captured variables)

--
Jon Skeet - <sk***@pobox.co m>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Marc Gravell]

I'm not sure that Sleep nor a loop would help here - really I suspect
all you need is:

for (int i = 0; i < 2; i++)
{
int j = i;
Thread t = new Thread(delegate ()
{
Debug.Assert(j != 2);
});
t.Start();
threads.Add(t);
}

By 1.1 standards it looks the same, but j should (IIRC) now be scoped
(and hence captured) *inside* the loop. On each successive loop, the j
is completely different. Without the capture, of course, there is only
a single j declared on the stack at the top of the method... gotta
love capture ;-p

Marc

[Marc Gravell]

ahh, frick; sorry Jon - I didn't see your (scarily similar) post.
Deferred posting... oops and apols...

[Peter Duniho]

Marc Gravell wrote:

I'm not sure that Sleep nor a loop would help here - really I suspect
all you need is:

Help with what? The post to which I replied asked if there was a way to
_ensure_ that the two different versions of the code behaved
differently. I don't know why he's unable to reproduce a difference
without changing the samples; assuming he's running on Windows, it's
hard to imagine a scenario where the thread creating the other threads
gets preempted before the i loop exits.

But taking as granted his statement is true and he does in fact have
trouble seeing the difference, there are ways to hack up the code so
that the timing is more deterministic.

Please don't think that the post to which you replied was intended to
address the original issue. My comments there are _strictly_ with
respect to manipulating the code so that it fails in a more obvious,
reproducible way.

Pete

[AJ]

As a follow-on question... where does variable i 'live'. It's local and
a value type, so lives on the local stack...

So... remove the thread.Join() and make the threads take a little longer
e.g.

new Thread( delegate() { Thread.Sleep(50 00); Debug.Assert(i! =2); }

The original method will have completed before the thread runs its code.
So where is the 'i' that it's referencing? Won't the stack have changed
by then?


In article <e6************ **@TK2MSFTNGP02 .phx.gbl>, cm******@yahoo. com
says...

I hit this bug last night in some test code I had written. It took a few
minutes to figure out what the root cause was, and it left me thinking,
"Wow. That was an interesting one that doesn't come up very often!".

So, for fun, who sees the bug and can explain why it's happening?

List<Threadthre ads = new List<Thread>();
for (int i = 0; i < 2; i++)
{
Thread t = new Thread(delegate ()
{
Debug.Assert(i != 2);
});

t.Start();
threads.Add(t);
}

foreach (Thread thread in threads)
thread.Join();

--
Chris Mullins

[Marc Gravell]

As a follow-on question... where does variable i 'live'. It's local

and
a value type, so lives on the local stack...

Nope, it doesn't live on the stack - it is "captured", so it lives as
a field on an object that the C# compiler creates for you. The rules
for the scoping of each "captured" variable is quite tricky... but but
*conceptually* what we are talking about (without using captures)
would be comparable to the following (although the compiler implements
it differently):

class SomeObject {
int i;
void SomeMethod() {
Debug.Assert(i != 2);
}
}
....
SomeObject obj = new SomeObject(); // obj instance is managed
for(int tmp = 0; tmp < 2; tmp ++) { // tmp lives on the stack
obj.i = tmp; // i lives on obj
Thread t = new Thread(obj.Some Method);
}

note that there is only a single "obj", and hence all share an "i".
Now compare to the version with a "j" introduced (see my other post in
this topic):

class SomeOtherObject {
int j;
void SomeOtherMethod () {
Debug.Assert(j != 2);
}
}
....
for(int i = 0; i < 2; i++) { // i lives on the stack
SomeOtherObject obj = new SomeOtherObject (); // obj instance is
managed
obj.j = i; // j lives on obj
Thread t = new Thread(obj.Some OtherObject);
}

in this latter case, each loop iteration gets a different object, and
hence a different "j"

Please note: I have simplified the behavior to make a simple example.

Marc

[Jon Skeet [C# MVP]]

On Oct 25, 10:47 am, AJ <no...@nowhere. comwrote:

As a follow-on question... where does variable i 'live'. It's local and
a value type, so lives on the local stack...

Well, it's only *sort* of a local variable. It's actually a captured
variable, and will live on the heap. It's the fact that it's captured
that makes the whole thing confusing.

Jon

[Mads Bondo Dydensborg]

Chris Mullins [MVP - C#] wrote:

Your explination is dead on, but the boxing / byref behavior of the
variable passed into the Closure is what made this code behavie in a way
other than was expected. I was expecting the int (a value type) to be
passed in by value, and therefore not change...

Actually, this functionality is much more useful. Besides the explanations
given here, this page may be interessting to you:

http://en.wikipedia.org/wiki/Closure_(computer_science)

Regards,

Mads

--
Med venlig hilsen/Regards

Systemudvikler/Systemsdevelope r cand.scient.dat , Ph.d., Mads Bondo
Dydensborg
Dansk BiblioteksCente r A/S, Tempovej 7-11, 2750 Ballerup, Tlf. [phone]
34