Is this a proper thread-safe Random wrapper?

Question

I am fairly inexperienced with threading and concurrency; to remedy that, I am currently working for fun on implementing a random-search algorithm in F#. I wrote a wrapper around the System.Random class, following ideas from existing C# examples - but as I am not sure how I would even begin to unit test this for faulty behavior, I'd like to hear what more experienced minds have to say, and if there are obvious flaws or improvements with my code, either due to F# syntax or threading misunderstanding:

open System
open System.Threading

type Probability() =

   static let seedGenerator = new Random()

   let localGenerator = 
      new ThreadLocal<Random>(
         fun _ -> 
            lock seedGenerator (
               fun _ -> 
                  let seed = seedGenerator.Next()
                  new Random(seed)))

   member this.Draw() = 
      localGenerator.Value.NextDouble()

My understanding of what this does: ThreadLocal ensures that for an instance, each thread receives its own instance of a Random, with its own random seed provided by a common, static Random. That way, even if multiple instances of the class are created close in time, they will receive their own seed, avoiding the problem of "duplicate" random sequences. The lock enforces that no two threads will get the same seed.

Does this look correct? Are there obvious issues?

Answer 1

I think your approach is pretty reasonable - using ThreadLocal gives you safe access to the Random and using a master random number generator to provide seeds means that you'll get random values even if you access it from multiple threads at similar time. It may not be random in the cryptographical sense, but should be fine for most other applications.

As for testing, this is quite tricky. If Random breaks, it will return 0 all the time, but that's just empirical experience and it is hard to say for how long you need to keep accessing it unsafely. The best thing I can suggest is to implement some simple randomness tests (some simple ones are on WikiPedia) and access your type from multiple threads in a loop - though this is still quite bad test as it may not fail every time.

Aside, you don't need to use type to encapsulate this behaviour. It can be written as a function too:

open System
open System.Threading

module Probability =

   let Draw =
     // Create master seed generator and thread local value
     let seedGenerator = new Random()
     let localGenerator = new ThreadLocal<Random>(fun _ -> 
       lock seedGenerator (fun _ -> 
         let seed = seedGenerator.Next()
         new Random(seed)))
     // Return function that uses thread local random generator
     fun () ->
       localGenerator.Value.NextDouble()