Search code examples
c#.netwinformstask-parallel-libraryparallel.foreach

Parallel.ForEach with retries not showing all the values


I implemented a sample application to see the behavior of the retries inside a Parallel.ForEach loop. As per my observations following application not showing all the values inside the array, in the textbox (showing character number changes randomly).

I want to know what cause this behavior. Thanks

public string DisplayText = "Numbers =";

private void button1_Click(object sender, EventArgs e)
{
    int[] numbers = { 1, 2, 3, 4, 5, 6, 7, 8, 9 };
    Start(numbers);
    textBox1.Text = DisplayText;
}

private void Start(int[] numbers)
{
    Parallel.ForEach(numbers, k =>
    {
        Write(k.ToString());
    });
}

private void Write(string text)
{
    int maxRetryCount = 3;
    int retryCount = 0;
    int retryInterval = 1000;

    while (retryCount < maxRetryCount)
    {
        if(retryCount == maxRetryCount-1)
        {
            DisplayText = DisplayText + "," + text;
            break;
        }

        Thread.Sleep(retryInterval);
        retryCount++;
    }
}

Solution

  • This line:

    DisplayText = DisplayText + "," + text;
    

    ...does not happen in one CPU instruction, so it can be interrupted. When it is interrupted, a thread's copy of DisplayText (stored as a reference) could become out of date, resulting in data loss.

    When sharing variables between threads, you must use thread-safe data structures. In this case you could use a ConurrentQueue.

    ConcurrentQueue<string> _displayText = new();
    string DisplayText => string.Join(",", _displayText);
    

    Then, instead of

    DisplayText = DisplayText + "," + text;
    

    Use

    _displayText.Enqueue(text);
    

    This data structure is designed to be thread safe and will allow multiple threads append to the queue without interfering with each other.

    You could also write your own thread-safe data structure, e.g. by using lock around your updates. This is very easy to get wrong.

    But the best way is to solve this sort of problem is simply to avoid any need for thread-safe data structures, by avoiding shared state. You can do this by rewriting Write as a pure function.

    IEnumerable<string> Write(string text)
    {
        int maxRetryCount = 3;
        int retryCount = 0;
        int retryInterval = 1000;
    
        while (retryCount < maxRetryCount)
        {
            if(retryCount == maxRetryCount-1)
            {
                yield return text;
                break;
            }
    
            Thread.Sleep(retryInterval);
            retryCount++;
        } 
    }
    

    This way you can splice together the strings after they are computed independently. Using LINQ you can do this in one line.

    var displayText = string.Join
    (
        ",", 
        Enumerable.Range(1, 9)
            .AsParallel()
            .SelectMany
            (
                x => Write(x.ToString())
            )
    );
    

    This latter technique is sometimes called "functional approach".