Thread Safety in VB.Net

Question

I'm trying to run a multi-threaded console app in VB and am having thread cross-over. Basically I want to run 5 threads, have them continually access a queue, process, and repeat until there's nothing left. When all threads have processed I want them to do something else. I'm attempting to use SyncLock to prevent multiple threads from accessing but it does not seem to be working. Any help would be appreciated!

Dim iThread As Integer
Dim manualEvents(4) As ManualResetEvent

Sub Main()
    For i = 0 To 4
        manualEvents(i) = New ManualResetEvent(False)
        ThreadPool.QueueUserWorkItem(AddressOf DoOne)
    Next

    For Each handle As WaitHandle In manualEvents
        handle.WaitOne()
    Next

    ' do other stuff
EndSub

Private Sub DoOne()
    Dim lockObject As New Object()
    SyncLock (lockObject)
        If QueryQueue.DoOne() Then
            DoOne()
        Else
            manualEvents(iThread).Set()
            iThread = iThread + 1
        End If
    End SyncLock
End Sub

Answer 1

The problem is with the locked resource, you're using lockObject as a synchronization lock resource which should be shared accros threads. You have to make it an instance field.

Private Shared lockObject As New Object()
Private Sub DoOne()
  SyncLock (lockObject)
    If QueryQueue.DoOne() Then
        DoOne()
    Else
        manualEvents(iThread).Set()
        iThread = iThread + 1
    End If
  End SyncLock
End Sub

Answer 2

The problem is that you are creating and using a new instance of an object for locking on each thread. The naive solution is to promote lockObject from a local variable to class variable. That way each thread is using the same object to lock on. I say this is naive because you have exchanged one problem for another (albeit less severe). The new problem is that you have now made your parallel algorithm a serial algorithm since only one thread can being doing work at any given time.

The solution would be to lock access to the queue only while it is being changed. Then operate on the dequeued objects outside the lock so that the threads can perform work concurrently.

If .NET 4.0 is available to you could refactored the code like this.

Public Class Example

  Private m_Queue As ConcurrentQueue(Of SomeObject) = New ConcurrentQueue(Of SomeObject)()

  Public Shared Sub Main()

    ' Populate the queue here.

    Dim finished = New CountdownEvent(1)
    For i As Integer = 0 to 4
      finsihed.AddCount()
      ThreadPool.QueueUserWorkItem(AddressOf DoOne, finished)
    Next
    finished.Signal()
    finished.Wait()

  End Sub

  Private Shared Sub DoOne(ByVal state As Object)
    Try
      Dim item As SomeObject = Nothing
      Do While m_Queue.TryDequeue(item) Then
        ' Process the dequeued item here.
      Loop
      ' There is nothing left so do something else now.
    Finally
      Dim finished = DirectCast(state, CountdownEvent)
      finished.Signal()
    End Try
  End Sub

End Class

I used ConcurrentQueue to avoid having to use SyncLock entirely. I used CountdownEvent as a more scalable alternative to wait for work items to complete.