Parallel programming with ManualResetEvent

Question

I have one method to handle a List<string> parallel. Unfortunately I can't use .NET 4+.

But when I run this method, i is always items.Count

public static void ParallelForEachTest(List<string> items)
{
    if (items != null && items.Count > 0)
    {
        List<ManualResetEvent> mEventList = new List<ManualResetEvent>();
        for (int i = 0; i < items.Count ; i++)
        {
            ManualResetEvent mEvent = new ManualResetEvent(false);
            ThreadPool.QueueUserWorkItem((y) =>
            {
                Console.WriteLine(items[i] + i);
                mEvent.Set();
            });
            mEventList.Add(mEvent);
        }
        mEventList.ForEach(x => x.WaitOne());
    }
}

What do I need to change to achieve

x 0
x 1
x 2

for ParallelForEachTest(new List<string>(){"x","x","x"});

Weird method. I think you should consider passing the value of i as a parameter into your parallelised code... — Ian, Sep 05 '16 at 13:01
Standard bug, it captures the for-loop variable in the lamba expression. [Read this](https://blogs.msdn.microsoft.com/ericlippert/2009/11/12/closing-over-the-loop-variable-considered-harmful/). By the time the thread starts running, the `i` variable has already been incremented. Usually. Use `var index = i;` and capture index. — Hans Passant, Sep 05 '16 at 13:14

Zoran Horvat · Accepted Answer · 2016-09-06T09:01:00.763

At the time when Console.WriteLine is executed, loop has already terminated and loop variable i has its final value, which is equal to list.Count.

In order to have each task print its corresponding index, you have to pass the index value to the task itself:

ThreadPool.QueueUserWorkItem((y) => { ... }, i);

The second argument is the state that will be passed to the callback once the task is started.

Also, make sure not to use closure to access state from inside the task. Only use task state to transfer data:

public static void ParallelForEachTest(List<string> items)
{
    if (items != null && items.Count > 0)
    {
        List<ManualResetEvent> mEventList = new List<ManualResetEvent>();
        for (int i = 0; i < items.Count; i++)
        {
            ManualResetEvent mEvent = new ManualResetEvent(false);
            ThreadPool.QueueUserWorkItem(
                (state) =>
                {
                    Tuple<string, int, ManualResetEvent> tuple =
                        (Tuple<string, int, ManualResetEvent>)state;
                    Console.WriteLine(tuple.Item1 + tuple.Item2);
                    tuple.Item3.Set();
                },
                Tuple.Create(items[i], i, mEvent));
            mEventList.Add(mEvent);
        }
        mEventList.ForEach(x => x.WaitOne());
    }
}

Thanks. To clarify, in C# 5 `foreach` was changed: in every iteration *new* variable is allocated. If you capture loop variable in a closure, C# 5 and later will give you new variable in each iteration. Prior to v 5, all iterations would capture the same variable and then with tasks that would fail. It is general advice not to capture loop variables because that may lead to strange behavior. — Zoran Horvat, Sep 06 '16 at 09:28

Parallel programming with ManualResetEvent

1 Answers1