1

I have the following code:

Thread[] threadArray= new Thread[3];
MyObject[] objectArray = new MyObject[3];
    for (int i = 0; i < 3; i++)
    {
      //Create new HotelSupplier object
      objectArray[i] = new MyObject();
      //Create array of threads and start a new thread for each HotelSupplier
      threadArray[i] = new Thread(new ThreadStart(objectArray[i].run));
      //Store the name of the thread
      threadArray[i].Name = i.ToString();
      //Start the thread
      threadArray[i].Start();
    }

I creating 3 objects and 3 threads. All of the objects are stored in an array, all of the threads are stored in an array.

The run method in MyObject generates a random number between min and max

Random random = new Random();
double min = 50.00;
double max = 500.00;
double price =  random.NextDouble() * (max - min) + min;
Console.WriteLine("GENERATING PRICE: " + price);

My output:

GENERATING PRICE: 101.271334780041
GENERATING PRICE: 101.271334780041
GENERATING PRICE: 101.271334780041

I expect 3 different random numbers instead of just one since each thread I thought would be running on a different object.

What am I doing wrong and what do I not understand about threads?

joshmmo
  • 1,062
  • 2
  • 13
  • 28
  • 1
    Try passing the RNG instance in, instead of creating a new one. You have just run into a *very* common problem – BradleyDotNET Sep 22 '14 at 22:20
  • 2
    The symptom you see has nothing to do with threading. Your `Random` created in each of your threads is being seeded by the same (time based) value. Extract it so it only gets seeded once. – Michael Sep 22 '14 at 22:20
  • 1
    Also, as currently stated this doesn't sound like a good use of threads. The overhead of creating the threads will most likely dwarf the time to generate the random values. Furthermore, writing to the console from different threads means synchronization which limits the level of parallelism. – Brian Rasmussen Sep 22 '14 at 22:23

2 Answers2

2

Your original code is causing all Random instances get seeded with the same value.

The lowest impact way for you to fix this would be to move the code to create the Random:

void Run()
{
    //...
    Random random = new Random();
    //...
}

outside of the Run method and into a private static variable, so it becomes:

private static Random random = new Random();
void Run()
{
    //...
}

so it is only seeded once across all instances of MyObject.

Michael
  • 3,099
  • 4
  • 31
  • 40
  • Thanks this solved my issue. Nice to know it had nothing to do with my threads. I will accept this answer as soon as I can. – joshmmo Sep 22 '14 at 22:30
  • 1
    If you use a shared random generator you must protect it with a lock to avoid race conditions. – Victor Victis Sep 22 '14 at 22:33
1

This is because Random is seeded with the current time, but the time resolution is usually of 15ms only. In other words the odds are very high that your three random generators have all been initialized during the same 15ms frame, that they all have the same seed and will therefore all yield the same value.

I suggest you create a first random generator on the initial thread, produce three random values, and use those as seeds for the other threads' generators.

Victor Victis
  • 313
  • 1
  • 8