Index overflow in for loop with nested thread

Question

I am writing a testing utility (a WinForm) to check how a web site perform. To do so i set a number of request to make, with a list of parameters associated with requests. I can set the requests to happen in parallel or in sequence.

If i work in sequence everything is fine, but if i work in parallel i get a strange issue with the for loop. I know i may use Parallel.For but because i am investigating another bug related to parallelism i temporary used a regualr for, with a nested action executed directly or with a Task.Run().

Here the problematic code :

private void Run()
{
    ConcurrentBag<long> callTimes = new ConcurrentBag<long>();
    int httpErrors = 0;
    int progress = 0;

    string uri = txtUrl.Text ?? string.Empty;
    if (string.IsNullOrWhiteSpace(uri))
        return;

    Func<List<string>,int,long> testCall = (p,i) =>
    {
        try
        {
            using (var client = new HttpClient())
            {
                Stopwatch timer = new Stopwatch();
                timer.Start();

                string actualUrl = string.Format(uri, p.ToArray());
                var getTask = client.GetAsync(actualUrl);
                getTask.Wait();

                timer.Stop();

                var result = getTask.Result;
                if (result == null || (int)result.StatusCode >= 400)
                {
                    txtErrors.ThreadSafeAppendText($"Connection error {(result?.StatusCode.ToString() ?? "NULL")}'\r\n");
                    Interlocked.Increment(ref httpErrors);
                }

                return timer.ElapsedMilliseconds;
            }
        }
        catch ( Exception actionErr)
        {
            txtErrors.ThreadSafeAppendText($"Error while execution callAction {i} with parameters '{string.Join(", " , p)}' : \r\n" + actionErr.Message);
        }

        return -1;
    };

    try
    {
        List<List<string>> parameters = this.ParseParameters();
        int parametersCount = parameters.Count;
        int executions = (int)updRequests.Value;

        //used to randomly access parameters in a way suitable also for the parallel scenario (i precompute all the random number i need while parallel processing is not yet started)
        Random rng = new Random();
        List<int> randoms = new List<int>();
        for (int i = 0; i < executions; i++)
            randoms.Add(rng.Next(0, parametersCount));

        //randoms.Count is guaranteed to be equal to executions 


        for ( int index = 0; index < executions; index++)
        {
            Action parallelAction = () =>
            {
                int currentIndex = index;
                List<string> currentParameter = parameters[randoms[currentIndex] % parametersCount]; //<<--- strange overflow here currentIndex >= executions
                callTimes.Add(testCall(currentParameter, currentIndex));

                Interlocked.Increment(ref progress);

                if (progress % 10 == 0)
                    prbProgress.ThreadSafeAction(this.RefreshProgressBar, progress, executions);
            };

            if (chkParallelExecution.Checked)
                Task.Run(parallelAction);
            else
                parallelAction();
        }


        this.Reporting(callTimes, httpErrors);
    }
    catch (Exception err)
    {
        txtErrors.ThreadSafeAppendText($"Error while running stress test : \r\n" + err.Message);
    }
}

The strange thing i don't understand is how the variable called currentIndex become >= executions variable, because only the loop manipulate those two variable and should enforce the opposite. So i think i am missing something in my understanding on how parallel processing happen here.

score 1 · Accepted Answer · answered Aug 10 '23 at 10:05

There is a fairly well known issue with capturing loop variables.

So you should probably write

for ( int index = 0; index < executions; index++)
{
    int currentIndex = index;
    Action parallelAction = () =>
    {
        ...

Another possible issue:

parameters[randoms[currentIndex] % parametersCount];

For all you know the values in randoms could all be zero. Are you sure you don't want to create an array of 0..executions, and shuffle this instead?

I can't see any obvious reasons why this example should fail however. But errors with indices etc should be rather obvious if you do some debugging.

Yep, i made a blunder when i tried to capture the variable. Thanks for the help — Skary, Aug 10 '23 at 10:29

Index overflow in for loop with nested thread

1 Answers1