3

can anyone please tell me, how is it possible that this code:

for (byte i = 0; i < someArray.Length; i++)
{
    pool.QueueTask(() =>
        {
            if (i > 0 && i < someArray.Length)
            {
                myFunction(i, someArray[i], ID);
            }
        });
}

falls on the line where myFunction is called with IndexOutOfRangeException because the i variable gets value equal to someArray.Length? I really do not understand to that...

Note: pool is an instance of simple thread pool with 2 threads.

Note2: The type byte in for loop is intentionally placed because the array length can not go over byte max value (according to preceding logic that creates the array) and I need variable i to be of type byte.

Jarda
  • 566
  • 1
  • 9
  • 33
  • Your array's not got more than 256 elements, has it? – Flynn1179 Nov 15 '15 at 17:16
  • @Flynn1179 it has 100 elements (that is why I use byte) – Jarda Nov 15 '15 at 17:44
  • byte is slower than int. http://stackoverflow.com/a/1148505/4767498 – M.kazem Akhgary Nov 15 '15 at 19:45
  • @M.kazemAkhgary well actualy, I use byte because of avoiding conversion from int to byte. The function takes first parameter byte and it serves as a data value for some data structure, which I store in binary file with custom layout - one byte, one long and so... So thank for effort, but that is not applicable for my case. – Jarda Nov 15 '15 at 20:09

2 Answers2

6

Your code is creating a closure on i, and it will end up being someArray.Length every time it's executed. The Action that you end up passing into QueueTask() retains the state of the for loop, and uses the value of i at execution time. Here is a compilable code sample that expresses this same problem,

static void Main(string[] args)
{
    var someArray = new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
    var fns = new List<Action>();

    for (int i = 0; i < someArray.Length; i++)
    {
        fns.Add(() => myFunction(i, someArray[i]));
    }

    foreach (var fn in fns) fn();
}

private static void myFunction(int i, int v)
{
    Console.WriteLine($"{v} at idx:{i}");
}

You can break this by copying the closed around variable in a local, which retains the value of i at creation time of the Action.

static void Main(string[] args)
{
    var someArray = new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
    var fns = new List<Action>();

    for (int i = 0; i < someArray.Length; i++)
    {
        var local = i;
        fns.Add(() => myFunction(local, someArray[local]));
    }

    foreach (var fn in fns) fn();
}

private static void myFunction(int i, int v)
{
    Console.WriteLine($"{v} at idx:{i}");
}

Related reading: http://csharpindepth.com/Articles/Chapter5/Closures.aspx

jdphenix
  • 15,022
  • 3
  • 41
  • 74
  • @Sehnsucht - Thank you! – jdphenix Nov 15 '15 at 17:27
  • 1
    There is actually a related breaking changing with C# 5.0 and the `foreach` loop, http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx that may interest you to read about as well. – jdphenix Nov 15 '15 at 18:01
1

Using a byte as index seems like a premature micro-optimization, and it will indeed cause trouble if your array has more than 255 elements.

Also, while we're at it: you mention that you're running on a threadpool. Are you making sure that the someArray does not go out of scope while the code is running?

snemarch
  • 4,958
  • 26
  • 38