C# Multiple processes generate the same random string-CodePudding

I have an array of strings. For each string I have to accomplish a task, and I want to do that in parallel. This is my piece of code:

    void Test()
    {
        sourcefiles.AsParallel().ForAll(MyMethod);
    }

    private void MyMethod(string source)
    {
       string tmp_string = RandomString(10);

       string path = Path.Combine(@"C:\myfolder", tmp_string);
       
      // (File operations on `path` go here)
    }

    private static Random random = new Random();

    public static string RandomString(int length)
    {
        const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

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

I receive the following exception: Cannot access C:\myfolder\ABCDEFG because it is in use by another process which is weird because the string "ABCDEFG" should be a random one generated by different processes. This is not an unfortunate event, it happens all the time. I also increased the complexity of RandomString(), but nothing happens, in fact I keep seeing from the debug that there are two different processes with the same tmp_string (other processes have a different one). Where's the mistake?

For context, this is what happens:

    PROCESS - RANDOM_STRING
    1937 - "r6MbODsNcF1654683907030"
    1374 - "MqrdQe386M1654683928872"  <---
    1518 - "iX33edEA5F1654683928873"
    1691 - "MqrdQe386M1654683928872"  <---
    1486 - "u46vqUrt601654684013613"

the first 10 alphanumeric characters are from the RandomString() function, the following digits represent the Unix's epoch

CodePudding user response：

Your RandomString implementation is using the parameterless constructor of Random, which, if created in quick succession, uses the same seed and therefore produces the same output.

CodePudding user response：

As you stated, you are running different processes with the above given code. And within this code you use a single static instance of Random. So far so good.

Unfortunately uses the Random class as initial seed the current time when created. So if you create multiple process in a tight loop, you got high chances, that two processes are using the same time as seed for the random class, which leads to producing the same numbers.

As you already shown in your code, it seems you already have access to the process id. So maybe you should generate your own seed value for the random class from the current time and the process id, maybe by calling something like this:

var seed = HashCode.Combine(DateTime.UtcNow, processId);
var random = new Random(seed);

Nevertheless, even if you can make a lower probability to hit the same value, you should check in your code if such a directory already exists and if yes, simply start a new try with the next random value.

And a last more simple trick:

If you don't care about the name of the folders, you can also use Guid.NewGuid().ToString() as your random folder name. The underlying implementation uses more sources for the seed then only time and the 128 bit range should move the probability low enough for your needs.

CodePudding user response：

The Random instance is created many times in short period of time, using current time as the seed. Effectively, many sequences of pseudorandom numbers generated this way will likely be identical.

To make each instance of Random create a different stream of random values, you need a random seed for them - such as a GUID:

private static Random random = new Random(Guid.NewGuid().GetHashCode());

In this particular code sample, the same Random instance was used concurrently, which it is not safe for. You can modify your RandomString method to create a fresh Random instance every time it is used:

public static string RandomString(int length)
{
    Random rnd = new Random(Guid.NewGuid().GetHashCode());
    const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";

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

CodePudding user response：

Random is not threadsafe! So your code is not correct since it uses a single random object from multiple threads concurrently. So the first step should be to ensure thread safety, either by using a lock or by creating a separate random object per thread. If you go for the later solution you need to ensure that the seed for each random object is different.