I've got an option in one of my programs to limit the number of threads that get used when the program loops through a list of computers to perform queries against them - I don't want to just use ThreadPool.QueueUserWorkItem because that could spawn too many threads and hammer the CPU (people often use the program to query several hundred remote PCs so I wouldn't want it trying to query all of them at once on separate threads). So I let the user specify how many threads should be allowed to run at once, but there seems to be a race condition that can sometimes cause the task to never finish.
I think this is due to the way I'm limiting the threads but I can't think of a way of fixing this. I'll demonstrate how it works now (I've taken out a lot of irrelevant code for this question, so don't worry if the overall purpose of it doesn't make sense, just focus on the thread counting and locking).
vb.net Code:
'At class level:
Private _ThreadLimitLock As New Object
Private _ThreadLimitSignal As New System.Threading.AutoResetEvent(False)
Private _CurrentThreadCount As Integer
Private _RemainingCountLock As New Object
Private _RemainingComputers As Integer
Private _AllFinishedSignal As New System.Threading.ManualResetEventSlim(False)
'In sub that performs the tasks:
For Each Computer In ComputerList
System.Threading.Interlocked.Increment(_CurrentThreadCount)
If _CurrentThreadCount > ThreadLimit Then
_ThreadLimitSignal.WaitOne()
End If
Dim BgThread As New System.Threading.Thread(AddressOf GetServicesFromSingleComputer)
BgThread.IsBackground = True
BgThread.Start(Computer)
Next
_AllFinishedSignal.Wait()
So it is looping through each computer and starting a thread for each of them, unless the current thread count is over the user specified limit - in which case it waits on the AutoResetEvent to be signalled. This gets signalled from the method that each background thread is running but the problem is I need to use a lock around the part that signals it... like so:
vb.net Code:
SyncLock _ThreadLimitLock
_CurrentThreadCount -= 1
If _CurrentThreadCount <= ThreadLimit Then
_ThreadLimitSignal.Set()
End If
End SyncLock
SyncLock _RemainingCountLock
_RemainingComputers -= 1
If _RemainingComputers < 1 Then
_AllFinishedSignal.Set()
End If
End SyncLock
I need this to all be done under a lock because multiple threads will be running this at the same time so I need them to decrement the count one at a time as they finish their work. Even if I changed this to use Interlocked.Decrement to decrement the counter without a lock, I'd still need to lock on the part that actually checks to see if the current thread count is now below the thread limit and signals the other thread to continue wouldn't I?
The problem as far as I can see, is that in the loop through the computer list we are incrementing the thread counter and checking its value then waiting on the AutoResetEvent but none of this is done under the lock that prevents multiple threads from modifying the thread counter at once. I can't make this be done under that lock though because then it is just going to deadlock and never proceed because one thread will be waiting for the lock so it can signal the AutoResetEvent but it will never get that lock because the thread that has the lock is sat waiting on the WaitOne method in the computer loop.
Any suggestions for how I can sort this out or alternatives to the way I'm doing it? All I want is to be able to control the number of threads I'm creating at any one time and have a way of knowing when they are all complete.
Thanks
Chris