Jarek Kożdoń
Jarek Kożdoń

Reputation: 336

Random.Next() working only for restricted time/number of invokes?

I've been using a code that runs "forever" with a static random in one of classes. All works fine, numbers are random and there's no problem... for a restricted time (or number of invokes, not sure). After some time it starts to generate the same number all the time.

And here's the question: do you know of any time limitations over random class? Have you ever had such a problem?

I've changed the code and I'll now in like a week or two if that helped (each x minutes I create new object), but I'd like to know what's behind that and what should I be more careful about in future.

OK... little update:

No, it's not about multithreading, it works fine for a long time (like a week or so) after that time it returns one number only all the time.

private static Random random = new Random();

public static string GetRandomString(int length)
{
    const string chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

    return new string(Enumerable.Repeat(chars, length)
        .Select(s => s[random.Next(s.Length)]).ToArray());
}

After "a time" (not sure how long) it starts to return the same string all over. as mentioned above: it's not a matter of multithreading as it continues to return the same value "forever".

Upvotes: 0

Views: 174

Answers (1)

Jarek Kożdoń
Jarek Kożdoń

Reputation: 336

OK, lets sum it up

  1. yes, multithread access was used to access that random. That didn't seem to be a problem though, as random text was returned and even if a few double texts were returned that was not a problem (should not happen)
  2. the problem with random and multithreading is not what people use to say: "don't do that because it can return same values", the reason is (mentioned by Evk in chat) the internal state of random resulting in losing "randomness" of random and even breaking the object causing the mentioned malfunction (returning 0 all the time)

  3. If you still don't believe or just want to know "How does it really work..." (just like me wanted) and you have some time to read... here you go: inside Random class there is an array

    private int[] SeedArray = new int[56];

when using any Next method there is InternalSample method called always and it looks like that:

 private int InternalSample()
    {
      int inext = this.inext;
      int inextp = this.inextp;
      int index1;
      if ((index1 = inext + 1) >= 56)
        index1 = 1;
      int index2;
      if ((index2 = inextp + 1) >= 56)
        index2 = 1;
      int num = this.SeedArray[index1] - this.SeedArray[index2];
      if (num == int.MaxValue)
        --num;
      if (num < 0)
        num += int.MaxValue;
      this.SeedArray[index1] = num;
      this.inext = index1;
      this.inextp = index2;
      return num;
    }

if you access that method by 2 threads at the same time there is a hazard that might cause inext to be incremented and inextp not to be incremented (usually both get +1) if this situation happens 21 times (initial difference between inext and inextp is 21) then this line:

int num = this.SeedArray[index1] - this.SeedArray[index2];

will start causing the seed array to be filled with 0s and after a short while random object will be completely broken returning 0s all the time.

Hazard explenation: intext = 0; inextp = 21 thread 1 (t1) and thread 2 (t2) access Random object. T1 is near to be finished and is at the line

this.inext = index1; //now this.inext = 1 and still this.inextp = 21 

t2 starts to work now and does

    int inext = this.inext; //1
    int inextp = this.inextp; //21!

t1 finishes with

this.inextp = index2; //22

after a while t2 finishes it's work and saves incremented local index1 and index2

        this.inext = index1; //2
        this.inextp = index2; //22!

as mentioned initial difference between this.inext and this.inextp was 21 - now it's only 20, do that 20 more times and random will start returning 0s all the time

  1. solution1: always lock random when there is possibility that it'll be used by multiple threads. solution2: that might be an interesting trick to copy disassembled random and write your own with internal lock object used at InternalSample() method (yeah, that will cause Random to work much slower but if you want to use the same object for multiple threads for any reason that would save problems with locking it every time externally).

PS i was pretty sure that multiple threads access to my random is very rare, but it wasn't, i've spotted a bug in code causing it to be accessed much to often by some clients. Thanks to that random error i could spot it and decrease load significantly. at least that was a minor reward.

Upvotes: 2

Related Questions