I have a class that has two method in it, one calls a class which creates and executes a number of threads, the other is an event handler that handles an event raised when those threads complete (and then calls the first method again).
I understand that the method that handles the event runs in the thread that raised the event. So as such, I SyncLock
a member variable that says how many threads are running and subtract one from it:
SyncLock Me ' GetType(me)
_availableThreads -= 1
End SyncLock
So I have a few questions:
Main Question: Should I be SyncLock'ing _availableThreads
everywhere in the class - i.e in the method that creates the threads (which adds 1 when a thread is created)
Side Questions related to this question:
I'd usually SyncLock
the current instance, but I've seen code that SyncLock
s the type instead, so what is the difference between sync locking Me
(Current Instance) and GetType(Me)
?
Would there be a performance difference between the two? and is there anything smaller I'd be able to lock for the above that doesn't affect anything else - perhaps a separate 'padlock' object created for the sole purpose of locking things within a class?
Note: The sole purpose of _availableThreads
is to control how many threads can run at any given time and the threads process jobs that can take hours to run.
Code:
Public Class QManager
Private _maxThreadCount, _availableThreads As Integer
Public Sub New(ByVal maxThreadCount As Integer)
Me.MaximumThreadCount = maxThreadCount
End Sub
Public Sub WorkThroughQueue()
//get jobs from queue (priorities change, so call this every time)
Dim jobQ As Queue(Of QdJobInfo) = QueueDAO.GetJobList
//loop job queue while there are jobs and we have threads available
While jobQ.Count > 0 And _availableThreads <= _maxThreadCount
//create threads for each queued job
Dim queuedJob As New QdJob(jobQ.Dequeue)
AddHandler queuedJob.ThreadComplete, AddressOf QueuedJob_ThreadCompleted
_availableThreads += 1 //use a thread up (do we need a sync lock here?)***************************
queuedJob.Process() //go process the job
End While
//when we get here, don't do anything else - when a job completes it will call this method again
End Sub
Private Sub QueuedJob_ThreadCompleted(ByVal sender As QdJobInfo, ByVal args As EventArgs)
SyncLock Me //GetType(me)
_availableThreads -= 1
End SyncLock
//regardless of how the job ended, we want to carry on going through the rest of the jobs
WorkThroughQueue()
End Sub
#Region "Properties"
Public Property MaximumThreadCount() As Integer
Get
Return _maxThreadCount
End Get
Set(ByVal value As Integer)
If value > Environment.ProcessorCount * 2 Then
_maxThreadCount = value
Else
value = Environment.ProcessorCount
End If
LogFacade.LogInfo(_logger, "Maximum Thread Count set to " & _maxThreadCount)
End Set
End Property
#End Region
End Class
You shouldn't SyncLock
the instance or the type. You always want to SyncLock
on a variable that is fully within the control of the class, and neither of those are. You should declare a private New Object
and use that for your SyncLock
.
Private lockObject as New Object()
...
SyncLock lockObject
...
End SyncLock