10

I have the following class

class Program
{
   static Random _Random = new Random();

   static void Main(string[] args)
   {
      ...
      for (int i = 0; i < no_threads; ++i)
      {
         var thread = new Thread(new ThreadStart(Send));
         thread.Start();
      }
      ...   
   }

   static void Send()
   {
      ...
      int device_id = _Random.Next(999999);
      ...
   }
}

The code creates the specified number of threads, starts each one, and assigns each thread a random device_id. For some reason, the first two threads that are created often have the same device_id. I can't figure out why this happens.

dandan78
  • 13,328
  • 13
  • 64
  • 78
  • This is statistically correct, as if you throw 2 dice they sometime give you the same number, plus Random is not thread safe -> http://blogs.msdn.com/b/pfxteam/archive/2009/02/19/9434171.aspx – dvhh Jan 06 '11 at 10:03
  • 3
    @Mitch: None of the linked questions are dealing with the same situation. It's only the thread safety which is relevant here, as there's only one instance of Random. – Jon Skeet Jan 06 '11 at 10:13
  • @Jon: yeah, that's true. – Mitch Wheat Jan 06 '11 at 10:15
  • 1
    By the sound of things, if two threads end up with the same device_id, it's causing you a problem. If that's the case, then you shouldn't be using *random* assignments anyway - you should do something that *guarantees* that there are no collisions. – Damien_The_Unbeliever Jan 06 '11 at 10:21
  • @dvvh: yeah, but the intraval is big enough so that a collission shouldn't happen every other run. The problem has been eliminated, tho. – dandan78 Jan 06 '11 at 11:15
  • @damien...: obviously, but this is just a proof of concept app anyway. – dandan78 Jan 06 '11 at 11:15

3 Answers3

27

Random is not thread-safe - you shouldn't be using the same instance from multiple threads. It can get much worse than just returning the same data - by using it from multiple threads, you can get it "stuck" in a state where it will always return 0, IIRC.

Obviously you don't just want to create a new instance for each thread at roughly the same time, as they'll end up with the same seeds...

I have an article which goes into the details of this and provides an implementation which lazily instantiates one instance of Random per thread using an incrementing seed.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • I suspected as much but wasn't sure even after checking MSDN. I'll check out your article as well. Thanks. – dandan78 Jan 06 '11 at 10:14
  • See also Jon Skeet's [MiscUtil.StaticRandom](http://www.yoda.arachsys.com/csharp/miscutil/). – Brian Jan 06 '11 at 14:40
5

Random is a pseudo-random number generator and there's nothing preventing it from returning same result for multiple calls. After all there's a probability for this happening. Not to mention that according to the documentation:

Any instance members are not guaranteed to be thread safe.

So you shouldn't be calling the Next method at all from multiple threads.

Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
2

Your example code only shows one use of _Random per thread. Assuming this is the case, you could also generate the random number in the main for loop and pass the random number into each thread as a parameter.

 for (int i = 0; i < no_threads; ++i)
 {
      var thread = new Thread(new ThreadStart(Send));
      thread.Start(_Random.Next(999999));
 }

and then modify your thread function to accept the parameter:

 static void Send(int device_id)
 {
    ...
    //int device_id = _Random.Next(999999);
    ...
 }
Grhm
  • 6,726
  • 4
  • 40
  • 64