Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Concurrency issues with Random in .Net?

I've debugging some problem with a Paint.Net plugin and I've stumbled with some issue with the Random class, when several threads call a method from a single instance.

For some strange reason, it seems that if I do not prevent concurrent access, by synchronizing the called method, my Random instance starts to behave... randomly (but in the bad sense).

In the following example, I create several hundred threads that call repeteadly a single Random object. And when I run it, I sometimes (not always, but nearly) get clearly wrong results. The problem NEVER happens if I uncomment the Synchronized method annotation.

using System;
using System.Threading;
using System.Runtime.CompilerServices;
namespace testRandom {

    class RandTest  {
        static int NTIMES = 300;

        private long ac=0;

        public void run() { // ask for random number 'ntimes' and accumulate
            for(int i=0;i<NTIMES;i++) {
                ac+=Program.getRandInt();
                System.Threading.Thread.Sleep(2);
            }
        }

        public double getAv() { 
            return ac/(double)NTIMES; // average
        }
    }

    class Program
    {

        static Random random = new Random();
        static int MAXVAL = 256;
        static int NTREADS = 200;

        //[MethodImpl(MethodImplOptions.Synchronized)]
        public static int getRandInt() { 
            return random.Next(MAXVAL+1); // returns a value between 0 and MAXVAL (inclusive)
        }

        public static void Main(string[] args)      {
            RandTest[] tests = new RandTest[NTREADS];
            Thread[] threads = new Thread[NTREADS];
            for(int i=0;i<NTREADS;i++)  {
                tests[i]= new RandTest();
                threads[i] = new Thread(new ThreadStart(tests[i].run));
            }
            for(int i=0;i<NTREADS;i++)  threads[i].Start();
            threads[0].Join();
            bool alive=true;
            while(alive) { // make sure threads are finished
                alive = false;
                for(int i=0;i<NTREADS;i++)  { if(threads[i].IsAlive) alive=true; }
            }
            double av=0;
            for(int i=0;i<NTREADS;i++)  av += tests[i].getAv();
            av /= NTREADS;
            Console.WriteLine("Average:{0, 6:f2}   Expected:{1, 6:f2}",av,MAXVAL/2.0);

            Console.Write("Press any key to continue . . . ");
            Console.ReadKey(true);
        }
    }
}

An example ouput (with the above values) :

Average: 78.98   Expected:128.00
Press any key to continue . . .

Is this some known issue? Is it incorrect to call a Random object from several threads without sync?

UPDATE: As per answers, the docs state that Random methods are not thread safe - mea culpa, I should have read that. Perhaps I had read that before but didn't think it so important - one could (sloppily) think that, in the rare event of two threads entering the same method concurrently, the worst that could happen is that those calls get wrong results - not a huge deal, if we are not too concerned about random number quality... But the problem is really catastrophic, because the object is left in an inconsistent state, and from that on it returns keeps returning zero - as noted here.

like image 331
leonbloy Avatar asked May 06 '11 18:05

leonbloy


3 Answers

For some strange reason

It's not really strange - Random is documented not to be thread-safe.

It's a pain, but that's life. See my article on Random for more information, and suggestions for how to have an instance per thread, with guards against starting with the same seed in multiple threads.

like image 138
Jon Skeet Avatar answered Oct 23 '22 03:10

Jon Skeet


The Random class is not thread safe.

From the docs:

Any instance members are not guaranteed to be thread safe

Instead of synchronizing which will cause all the threads to block, try implementing the ThreadStatic attribute.

like image 28
The Scrum Meister Avatar answered Oct 23 '22 03:10

The Scrum Meister


Random isn't guaranteed to be thread safe: http://msdn.microsoft.com/en-us/library/system.random.aspx unless it's public static.

like image 45
James Michael Hare Avatar answered Oct 23 '22 02:10

James Michael Hare