Skip to content

Bug: Acolyte.Basic.Randomness.ThreadSafeRandom class can have the same seed value when created simultaneously using default ctor #77

Description

@Vasar007

Describe the bug
System.Random class is not thread-safe. So, we should refactor Acolyte.Basic.Randomness.ThreadSafeRandom to use singleton System.Random class for seed generation to avoid the issue when several Acolyte.Basic.Randomness.ThreadSafeRandom class created simultaneously (using default ctor = seed will be generated randomly) will have the same seed value.

Read about it more here.

To Reproduce

Example from the article mentioned above:

using System;
using System.Linq;
using System.Threading.Tasks;
using Acolyte.Basic.Randomness;

// ⚠ This isn't safe in .NET Framework, don't do it ⚠
Parallel.For(0, 10, x =>  // run in parallel
{
    TheadSafeRandom rng = new(); // 👈 create a private TheadSafeRandom instance
    var numbers = new int[10_000];
    for (int i = 0; i < numbers.Length; ++i)
    {
        numbers[i] = rng.Next(); // Fetch 10,000 random numbers
    }

    var numZeros = numbers.Count(x => x == 0); // how many issues were there?
    Console.WriteLine($"Received {numZeros} zeroes"); // always 0 issues
});

Expected behavior
Acolyte.Basic.Randomness.ThreadSafeRandom class should use singleton Random class for seed generation.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area: C# LibraryRelated to the C# part of librarytype: BugSomething isn't working

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions