Friday, July 22, 2005

I made a pretty simple mistake just now. I knew the source of the problem immediately when I saw the exception, but it's fairly interesting.

What's wrong with this code?

MyComponent[] myComponents = Create25Components();
for (int i = 0; i < 25; i++) {
    Thread t = new Thread(delegate () {
        // do some stuff
        myComponent[i].DoWork();
        // do some more stuff
    });
    t.Start();
}

Oops! you say? Oops! for sure.

The reference to the induction variable i gets treated like an ordinary variable C# captures inside an anonymous delegate closure. Namely that it just gets captured into a closure class which each thread shares access to. Access to i inside the thread simply dereferences the shared memory location to obtain the value during execution... not when you capture it. So assuming the parent thread is able to spin through the loop quickly, all of your threads will probably see the final result of i, which is 25. It turns out 25 is an invalid index for the array, resulting in an IndexOutOfRangeException or two. If they get a chance to run quickly, they will see some number in between, but probably not the correct one!

One (of many) solutions is to write this instead:

MyComponent[] myComponents = Create25Components();
for (int i = 0; i < 25; i++) {
    MyComponent mc = myComponents[i];
    Thread t = new Thread(delegate () {
        // do some stuff
        mc.DoWork();
        // do some more stuff
    });
    t.Start();
}

Easy mistake.

 

RSS 2.0

Me
 

Joe Send mail to the author(s) is an architect and developer on a systems incubation project at Microsoft.

Recent

Search

Browse

Disclaimer:
The content of this site are my own personal opinions and do not represent my employer's view in anyway.

© 2013, Joe Duffy