Где и чем синхронизировать блокировку в этом коде - PullRequest
0 голосов
/ 14 января 2011

У меня есть класс, в котором есть два метода: один вызывает класс, который создает и выполняет несколько потоков, другой - обработчик событий, который обрабатывает событие, возникающее после завершения этих потоков (а затем снова вызывает первый метод).).

Я понимаю, что метод, который обрабатывает событие, выполняется в потоке, который вызвал событие.Поэтому я синхронизирую переменную-член, которая сообщает, сколько потоков запущено, и вычитает из нее один:

SyncLock Me //GetType(me)
    _availableThreads -= 1
End SyncLock

Поэтому у меня есть несколько вопросов:

Основной вопрос: Должен ли я использовать SyncLock _availableThreads везде в классе - т.е. в методе, который создает потоки (который добавляет 1 при создании потока)

Дополнительные вопросы, связанные с этим вопросом:

  1. Обычно я бы синхронизировал текущий экземпляр, но я видел код, который вместо этого синхронизирует тип, так в чем же разница между синхронизацией Me (текущий экземпляр) и GetType(me)?

  2. Будет ли разница в производительности между ними?и есть ли что-то меньшее, что я мог бы заблокировать для вышеперечисленного, которое ни на что не влияет - возможно, отдельный объект «навесной замок», созданный с единственной целью блокировки вещей внутри класса?

Примечание. Единственная цель _available потоков - контролировать, сколько потоков может выполняться в любой момент времени, и потоки обрабатывают задания, выполнение которых может занять часы.

Код:

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

Ответы [ 4 ]

6 голосов
/ 14 января 2011

Вы не должны SyncLock экземпляр или тип. Вы всегда хотите SyncLock для переменной, которая полностью находится под контролем класса, и ни одна из них не является Вы должны объявить личный New Object и использовать его для своего SyncLock.

Private lockObject as New Object()

...

SyncLock lockObject
   ...
End SyncLock
2 голосов
/ 14 января 2011

К сожалению, здесь нужно сделать несколько вещей по-другому.

Прежде всего, я бы рекомендовал избегать SyncLock и использовать Interlocked.Increment и Interlocked.Decrement для обработки изменяющихся _availableThreads.Это обеспечит безопасность потока для этой переменной без блокировки.

При этом вам все равно понадобится SyncLock для каждого доступа к вашей очереди - если он используется из нескольких потоков.Альтернативой, если вы используете .NET 4, было бы переключиться на использование нового класса ConcurrentQueue (Of T) вместо очереди.Если вы используете SyncLock, вы должны создать частный объект, доступный только вашему классу, и использовать его для всей синхронизации.

2 голосов
/ 14 января 2011

Вы должны использовать класс Interlocked здесь, метод Decrement (), чтобы уменьшить количество.Да, везде, где доступна переменная.

Использование SyncLock Me так же плохо, как и SyncLock GetType (Me).Вы должны всегда использовать закрытый объект для блокировки, чтобы никто не мог случайно вызвать тупик.Золотое правило заключается в том, что вы не можете заблокировать данные, вы можете заблокировать доступ к данным только коду.Поскольку код является вашей частной деталью реализации, объект, который содержит состояние блокировки, также должен быть частной деталью.Ни ваш объект (Я), ни Тип этого объекта не являются частными.Разрешение другому коду блокировать его случайно.

1 голос
/ 14 января 2011

Вы можете заменить счетчик потоков на Семафор. Если вы используете семафор, вам не нужно выходить из цикла while, а также нет необходимости вызывать WorkThroughQueue () из обработчика событий ThreadCompleted. Семафор является поточно-ориентированным, поэтому вы можете использовать его без блокировки.

http://www.albahari.com/threading/part2.aspx#_Semaphore

...