AccessViolationException with Parallel.For()

555 Views Asked by At

The code snippet below exhibits a bug I was able to isolate from production code. The program will crash with a System.AccessViolationException.

My system configuration is Visual Studio 2013 on Windows 8.1 x64. To reproduce the bug, follow these steps in Visual Studio:

  1. Create an empty Console Application
  2. Replace the entire contents of Program.cs with the code below
  3. Compile in Release mode, Any CPU
  4. Start without debugging (CTRL + F5)

The program will crash immediately, displaying an error stack trace in the Console window.

I was not able to reduce the code any further, i.e. modifying any portion of the code will make the bug disappear.

Is there any way to tell the root cause of this bug? What would be a good workaround?

using System;
using System.Threading.Tasks;

namespace ParallelBugTest
{
    class Program
    {
        private class Value
        {
            public double X;
        }

        private class Aggregator
        {
            private Value myval = new Value();

            public void Add()
            {
                // use Min() instead of Max() -> bug disappears
                myval.X = Math.Max(0, myval.X);
            }
        }

        public static void Main(string[] args)
        {
            Parallel.For(0, 10000, Process);
        }

        private static void Process(int k)
        {
            Value[] V = new Value[10000];
            Aggregator aggregator = new Aggregator();

            for (int i = 0; i < V.Length; i++)
            {
                V[i] = new Value();
                aggregator.Add();
            }
        }
    }
}
2

There are 2 best solutions below

2
Matthias Müller On

Math.Max is not Thread-Safe. I guess Min is working because it is faster to calculate. Locking the Max-Call works:

    private static Object lockObj = new Object();

    private class Value
    {
        public double X;
    }

    private class Aggregator
    {
        private Value myval = new Value();

        public void Add()
        {
            // use Min() instead of Max() -> bug disappears
            lock (lockObj)
            { 
                myval.X = Math.Max(0, myval.X);
            }
        }
    }

    public static void Main(string[] args)
    {
        Parallel.For(0, 10000, Process);
    }

    private static void Process(int k)
    {
        Value[] V = new Value[10000];
        Aggregator aggregator = new Aggregator();

        for (int i = 0; i < V.Length; i++)
        {
            V[i] = new Value();
            aggregator.Add();
        }
    }

Fast and easier way is just to use an ordinary iif:

        public void Add()
        {
            // use Min() instead of Max() -> bug disappears
            myval.X = myval.X > 0 ? myval.X : 0;
        }
2
svick On

Everything in your code is thread-safe, since there is no shared state between threads (each iteration has its own Aggregator and the array of Values and you're not using any static fields).

Even if your code wasn't thread-safe, it doesn't do anything unsafe (i.e. working directly with memory), which should be the only way to get AccessViolationException.

Because of that, I believe this is a bug in the CLR and you should report it (click on the "submit feedback" link after logging in).