HighTechTalks DotNet Forums  

Spot the Bug: Fun Concurrency Bug

Dotnet Framework microsoft.public.dotnet.framework


Discuss Spot the Bug: Fun Concurrency Bug in the Dotnet Framework forum.



Reply
 
Thread Tools Search this Thread Display Modes
  #11  
Old   
Kalpesh
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 05:48 PM






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.Collections.Generic;
using System.Diagnostics;
using System.Threading;

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

Console.WriteLine("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] <sk... (AT) pobox (DOT) com> wrote:
Quote:
Chris Mullins [MVP - C#] <cmull... (AT) yahoo (DOT) com> wrote:



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<Thread> threads = 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<Thread> threads = 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 - <sk... (AT) pobox (DOT) com>http://www.pobox.com/~skeet Blog:http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too



Reply With Quote
  #12  
Old   
Willy Denoyette [MVP]
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 05:49 PM






"Jon Skeet [C# MVP]" <skeet (AT) pobox (DOT) com> wrote

Quote:
Chris Mullins [MVP - C#] <cmullins (AT) yahoo (DOT) com> wrote:
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<Thread> threads = 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<Thread> threads = 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((ParameterizedThreadStart)delegate(object
y){ Debug.Assert((int)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((int)y != 2); });
t.Start(i);
threads.Add(t);
}


Willy.



Reply With Quote
  #13  
Old   
Peter Duniho
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:03 PM



Kalpesh wrote:
Quote:
Could you explain, what is the problem with having to introduce j as a
temp. variable?
I don't think there's a problem introducing j as a temporary variable.
It's a nice solution to the problem.

Quote:
What is the problem with the original code (without introduction of
temp. variable inside the loop)?
The issue has to do with the order in which the code is executed and
what value each variable holds. The code you posted is likely (but not
guaranteed) to output something like this:

starting thread: 0
starting thread: 1
inside thread: 0
inside thread: 1

However, if you remove the local j variable, and in the anonymous method
just use i, you would get something like this:

starting thread: 0
starting thread: 1
inside thread: 2
inside thread: 2

Again, no guarantees due to thread scheduling, but it's a likely
outcome. And of course, the assertion would fail so in addition to the
output, you'd get that as well.

Hope that helps.

Pete


Reply With Quote
  #14  
Old   
Kalpesh
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:08 PM



Pete,

Thanks for your reply.
Pardon my ignorance - but it outputs the same thing on the machine
that I am on.

If that's the case, how to really see them outputting different
results (in order to produce the bug)?
Also, in real life case -how to make sure that problems like this dont
pass by & get caught when in production?

Thanks
Kalpesh


Reply With Quote
  #15  
Old   
Jon Skeet [C# MVP]
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:11 PM



Kalpesh <shahkalpesh (AT) gmail (DOT) com> wrote:

<snip>

Quote:
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 - <skeet (AT) pobox (DOT) com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too


Reply With Quote
  #16  
Old   
Peter Duniho
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:22 PM



Kalpesh wrote:
Quote:
Pete,

Thanks for your reply.
Pardon my ignorance - but it outputs the same thing on the machine
that I am on.
What outputs the same thing? What is the actual output? For which
version of the code?

Quote:
If that's the case, how to really see them outputting different
results (in order to produce the bug)?
The easiest thing would be to include a call to Thread.Sleep() at the
beginning of the anonymous method, to ensure that the rest of the code
in the anonymous method doesn't execute until the main thread has
finished initializing all of the threads.

An alternative would be to use an additional loop. Currently the code
has two: one initializes and starts each thread, while the second waits
for each to complete. You could break the first loop into two different
loops, one of which initializes each thread, another to actually start
each thread. That would reproduce the problem reliably as well.

Quote:
Also, in real life case -how to make sure that problems like this dont
pass by & get caught when in production?
Well, as a start: very VERY suspicious any time you use a variable
inside an anonymous method that was declared outside that anonymous
method, and the anonymous method is going to be executed asynchronously
relative to the code that's declaring the anonymous method.

The most common reasons for using an anonymous method involve it being
executed asynchronously, so this winds up meaning that you should simply
be suspicious any time a variable is used inside an anonymous method.

One notable exception to this would be using an anonymous method in a
call to Control.Invoke(). But otherwise, asynchronous scenarios are
typical.

By "suspicious", I don't mean you shouldn't do it. I just mean that you
should consider that to be an enormous red flag, requiring very careful
inspection of the code so that you understand exactly what it's doing.

IMHO, if you take the time to really stop and think about what
"capturing" a variable means, then any time you write an anonymous
method this will be very much in the front of your mind. I think
there's a tendency to read and write code in anonymous methods as if the
captured variable exists in the same context as the declaration of the
anonymous method. But it doesn't have to be that way; just remember
that the captured variable's value is used at the same time as the rest
of the code in the anonymous method is _executed_, and you should be
able to avoid these problems.

Pete


Reply With Quote
  #17  
Old   
Marc Gravell
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:40 PM



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


Reply With Quote
  #18  
Old   
Marc Gravell
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 06:42 PM



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


Reply With Quote
  #19  
Old   
Peter Duniho
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-24-2007 , 07:07 PM



Marc Gravell wrote:
Quote:
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


Reply With Quote
  #20  
Old   
AJ
 
Posts: n/a

Default Re: Spot the Bug: Fun Concurrency Bug - 10-25-2007 , 05:47 AM




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(5000); 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 <e6FVjpmFIHA.2268 (AT) TK2MSFTNGP02 (DOT) phx.gbl>, cmullins (AT) yahoo (DOT) com
says...
Quote:
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<Thread> threads = 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




Reply With Quote
Reply




Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off



Powered by vBulletin Version 3.5.4
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.