Thread Pooling with Arrays in C#, index is out of range

Question

I have been trying to use thread pooling on image filtering where the method takes out the red color from an image and returns the filtered image. I get an error when y variable reaches the max amount. I have been looking for answers but couldn't find anything related to this.

public Color[,] Apply(Color[,] input)
{
    int width = input.GetLength(0);
    int height = input.GetLength(1);
    Color[,] result = new Color[width, height];

    for (int x = 0; x < width; x++)
    {
        for (int y = 0; y < height; y++)
        {
            ThreadPool.QueueUserWorkItem(state => Work(x, y));
        }
    }

    void Work(int x, int y)
    {
        var p = input[x, y];
        result[x, y] = Color.FromArgb(p.A, 0, p.G, p.B);
    }

    return result;
}

"[How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/)" — Uwe Keim, Jan 23 '20 at 13:34
This isn't the cause of your current problem, but note that you return `result` potentially before all of the jobs you posted to `ThreadPool.QueueUseWorkItem` have completed — canton7, Jan 23 '20 at 13:34

score 5 · Accepted Answer · answered Jan 23 '20 at 13:34

5

Here:

ThreadPool.QueueUserWorkItem(state => Work(x, y));

x and y are captured - when the task runs (at some point in the future), they (x and y) won't have the values you're thinking of - they will have moved on. Define your own locals in the same scope:

var xx = x;
var yy = y;
ThreadPool.QueueUserWorkItem(state => Work(xx, yy));

answered Jan 23 '20 at 13:34

Marc Gravell

1,026,079
266
2,566
2,900

1

Specifically, at some point, `y` will be captured and then incremented so that it's equal to `height`. When `Work` executes, `y` will be `height`, and this causes your exception – canton7 Jan 23 '20 at 13:37
I tried this before and got an error. Now, I try again after you mentioned it and it worked. Software is definitly magic. I appreciate your effort. Have a nice day! – Erkan GÖRGÜLÜ Jan 23 '20 at 14:05
@ErkanGÖRGÜLÜ where exactly you declare he locals is very important. Declaration scope defines lifetime (so: which values are shared) for capture contexts. – Marc Gravell Jan 23 '20 at 14:25
@ErkanGÖRGÜLÜ This is racy, as well. If `Work` is executed before the next loop iteration, you're fine. If you get a loop iteration which pushes `y` up to `height` before `Work` executes, then you've got a problem. – canton7 Jan 23 '20 at 15:25
@canton7 since we're snapshotting inside the innermost part of the loop, I'm not sure what you mean there – Marc Gravell Jan 23 '20 at 20:13
1

@Marc I mean, the code in the question, without your fix. That wasn't at all clear, in hindsight... – canton7 Jan 23 '20 at 20:25

Thread Pooling with Arrays in C#, index is out of range

1 Answers1