[RESOLVED] Random Bug

This weekend I watched a video on YouTube(see comments for link) that said there is a bug in the Random class when used with threading. It was in C# so I thought I'd recreate it in VB. I was surprised to find it does fail. And after failing the first time it is like the Random is broken.

To use this code create a Form with three buttons and two labels with default names.

Button 1 is a sanity check and only fails after Button 2 fails.

Code:

---

    ' Is this  https://www.youtube.com/watch?v=WRB4OHpSXHs
    '  true?
    ' .Net 4.8
    '
    ' Strictly speaking zero values aren't errors.
    '  Documentation says this about .Next
    '  A 32-bit signed integer that is greater than or equal to 0 and less than Int32.MaxValue.
    ' but ...

    Private Shared PRNG As New Random
    Private LRand As Concurrent.BlockingCollection(Of Integer)
    Const Tries As Integer = 100000
    Const Thrds As Integer = 25
    Private STPW As New Stopwatch
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        SetUp()

        For x As Integer = 1 To Tries * Thrds
            LRand.Add(PRNG.Next)
        Next
        STPW.Stop()
        Dim Nzro As Integer
        Nzro = (From i In LRand Where i > 0 Select i).Count
        Dim zct As Integer = LRand.Count
        If Nzro <> zct Then
            Label1.Text = "1 ERR  " & (Nzro / zct).ToString("p1") 'per cent of non-zero values
        Else
            Label1.Text = "Good  " & Nzro.ToString("n0")
        End If
        Label2.Text = STPW.Elapsed.TotalMilliseconds.ToString("n1")
    End Sub

    Private Sub Button2_Click(sender As Object, e As EventArgs) Handles Button2.Click
        SetUp()

        Parallel.For(1, Thrds + 1,
                    Sub(idx As Integer)
                        For zx As Integer = 1 To Tries
                            Do While Not LRand.TryAdd(PRNG.Next)
                                Threading.Thread.Sleep(0)
                            Loop
                        Next
                    End Sub)
        STPW.Stop()
        Dim Nzro As Integer
        Nzro = (From i In LRand Where i > 0 Select i).Count
        Dim zct As Integer = LRand.Count
        If Nzro <> zct Then
            Label1.Text = "2 ERR  " & (Nzro / zct).ToString("p1") 'per cent of non-zero values
        Else
            Label1.Text = "Good  " & Nzro.ToString("n0")
        End If
        Label2.Text = STPW.Elapsed.TotalMilliseconds.ToString("n1")
    End Sub

    Private Sub SetUp()
        ' PRNG = New Random
        Label1.Text = ""
        LRand = New Concurrent.BlockingCollection(Of Integer)
        STPW.Restart()
    End Sub

---

Here is what I did to fix? this issue,

Code:

---

    Private Sub Button3_Click(sender As Object, e As EventArgs) Handles Button3.Click
        SetUp()

        Parallel.For(1, Thrds + 1,
                    Sub(idx As Integer)
                        For zx As Integer = 1 To Tries
                            Do While Not LRand.TryAdd(MyPRNG.NextI)
                                Threading.Thread.Sleep(0)
                            Loop
                        Next
                    End Sub)
        STPW.Stop()
        Dim Nzro As Integer
        Nzro = (From i In LRand Where i > 0 Select i).Count
        Dim zct As Integer = LRand.Count
        If Nzro <> zct Then
            Label1.Text = "3 ERR  " & (Nzro / zct).ToString("p1") 'per cent of non-zero values
        Else
            Label1.Text = "Good  " & Nzro.ToString("n0")
        End If
        Label2.Text = STPW.Elapsed.TotalMilliseconds.ToString("n1")
    End Sub

    Private Class MyPRNG
        Private Shared Block As New Threading.SpinLock '(True)
        Private Shared _PRNG As New Random

        Public Shared Sub NextB(buffer() As Byte)
            Block.Enter(Nothing)
            _PRNG.NextBytes(buffer)
            If Block.IsHeld Then
                Block.Exit()
            End If
        End Sub

        Public Shared Function NextD() As Double
            Block.Enter(Nothing)
            NextD = _PRNG.NextDouble
            If Block.IsHeld Then
                Block.Exit()
            End If
        End Function

        Public Shared Function NextI() As Integer
            Block.Enter(Nothing)
            NextI = _PRNG.Next
            If Block.IsHeld Then
                Block.Exit()
            End If
        End Function

        Public Shared Function NextI(maxValXclu As Integer) As Integer
            Block.Enter(Nothing)
            NextI = _PRNG.Next(maxValXclu)
            If Block.IsHeld Then
                Block.Exit()
            End If
        End Function

        Public Shared Function NextI(minValIlu As Integer, maxValXclu As Integer) As Integer
            Block.Enter(Nothing)
            NextI = _PRNG.Next(minValIlu, maxValXclu)
            If Block.IsHeld Then
                Block.Exit()
            End If
        End Function
    End Class

---

Not sure what the bug is, however the official documentation at https://learn.microsoft.com/en-us/do...et-6.0#remarks indicates that it isn't thread safe.

Quote:

---

Originally Posted by PlausiblyDamp

Not sure what the bug is, however the official documentation at https://learn.microsoft.com/en-us/do...et-6.0#remarks indicates that it isn't thread safe.

---

You are correct. The documentation even says, "If you don't ensure that the Random object is accessed in a thread-safe way, calls to methods that return random numbers return 0."

The 'bug' is that it actually breaks the instance of random being used. So in the code I posted Button 1 should not fail, but if you click on Button 2 until it fails Button 1 will also fail.

Maybe it isn't a bug but incomplete documentation.

From a pedantic point of view, the documentation isn't actually wrong. It certainly leaves a bit open to interpretation, but that's largely because they aren't all that specific about what the problem is and what the symptoms are.

For a long time, I thought that MS did a good job with documentation, and for the most part, I still feel that way when it comes to Visual Studio documentation. However, I have come to realize that this is not some organizational standard. Documentation standards don't exist for them at a company level. Instead, documentation standards are group by group, and even within a group the standards are loosely held and loosely defined.

Documentation is tough, good documentation is tougher.

Quote:

---

Originally Posted by Shaggy Hiker

From a pedantic point of view, the documentation isn't actually wrong. It certainly leaves a bit open to interpretation, but that's largely because they aren't all that specific about what the problem is and what the symptoms are.

For a long time, I thought that MS did a good job with documentation, and for the most part, I still feel that way when it comes to Visual Studio documentation. However, I have come to realize that this is not some organizational standard. Documentation standards don't exist for them at a company level. Instead, documentation standards are group by group, and even within a group the standards are loosely held and loosely defined.

Documentation is tough, good documentation is tougher.

---

I agree. They actually do a good job with this I think IF you follow the rules. It was the breaking, only returns zero values, if you don't that threw me.

https://learn.microsoft.com/en-us/do...8#ThreadSafety

Quote:

---

Originally Posted by dbasnett

You are correct. The documentation even says, "If you don't ensure that the Random object is accessed in a thread-safe way, calls to methods that return random numbers return 0."

The 'bug' is that it actually breaks the instance of random being used.

---

The documentation also says this about the example code provided:

Quote:

---

The example checks whether the random number generator has become corrupted by determining whether two consecutive calls to random number generation methods return 0.

---

That indicates that the Random instance being broken is the expected behaviour and not a bug. The reason that zero is returned in the first place is that something has been broken so expecting that to become unbroken seems a bit unreasonable.

You'd think I would know better than to believe everything I see on the internet. At least this is confirmation that not paying attention to thread safety documentation can get you into trouble and make you appear foolish. I certainly accomplished the latter.