Это правильный объект для блокировки потока безопасности?

Question

Код ниже будет вызываться с веб-сайта, поэтому наличие статического словарного объекта в нестатическом классе должно быть поточно-ориентированным.По сути, целью кода является инкапсуляция логики и сохранение времени жизни счетчика (ов) perfmon, которые хранятся в экземпляре CounterContainer.Конструктор называется передачей в instanceName.Конструктор должен проверить, был ли CounterContainer для этого instanceName определен и сохранен в словаре.Если это так, он может (и должен) использовать этот экземпляр.Если нет, то он создает экземпляр CounterContainer, сохраняет его в словаре и затем использует этот экземпляр.Используемый экземпляр CounterContainer хранится в нестатическом члене, поэтому с этого момента он безопасен для потоков.

Поскольку единственное место, в котором используется статический словарь, находится внутри конструктора, мне кажется, чтобудет безопасно заблокировать словарь на время доступа к нему?Будет ли это позже вызывать какие-либо непредвиденные проблемы с блокировкой / взаимоблокировкой?Я ничего не вижу, но в прошлом мне не приходилось слишком много думать об этом.

Я также рассмотрел lock (this): , но я не думаю, что это сработало бы, так как это блокировало бы только создаваемый экземпляр PerformanceCounters, а не базовый статический словарь (такне будет потокобезопасным).

namespace ToolKit
{
    using System;
    using System.Diagnostics;
    using System.Collections.Generic;

    public class PerformanceCounters : IPerformanceCounters
    {
        private static Dictionary<string, CounterContainer> _containers = new Dictionary<string, CounterContainer>();
        private CounterContainer _instanceContainer;

        public PerformanceCounters(string instanceName)
        {
            if (instanceName == null) throw new ArgumentNullException("instanceName");
            if (string.IsNullOrWhiteSpace(instanceName)) throw new ArgumentException("instanceName");

            // Is this the best item to lock on?
            lock (_containers)
            {

                if (_containers.ContainsKey(instanceName))
                {
                    _instanceContainer = _containers[instanceName];
                    return;
                }

                _instanceContainer = new CounterContainer(instanceName);
                _containers.Add(instanceName, _instanceContainer);
            }
        }
        public void Start()
        {
            _instanceContainer.AvgSearchDuration.Start();
        }

        public void FinishAndLog()
        {
            _instanceContainer.SearchesExecuted.Increment();
            _instanceContainer.SearchesPerSecond.Increment();
            _instanceContainer.AvgSearchDuration.Increment();
        }
    }
}

Answer 1

Не для того, чтобы не согласиться с предложением использовать ConcurrentDictionary, но для более общего ответа:

1. Лучшее решение для блокировки - это решение, которое вообще не блокируется. Иногда не очень сложно сделать что-то параллельное без блокировки. Далее лучше всего иметь мелкозернистые замки. Тем не менее, грубозернистые замки гораздо проще рассуждать и, следовательно, быть уверенными в их правильности. Если у вас нет хорошо протестированного класса без блокировок соответствующего назначения, начните с грубых блокировок (блокировок, которые блокируют целую кучу операций с одной и той же блокировкой), а затем переходите к более мелкозернистой либо потому, что есть логический тупик (вы можете видеть, что две несвязанные операции могут блокировать друг друга) или в качестве оптимизации при необходимости.
2. Никогда не блокируйте this, потому что что-то внешнее по отношению к вашему коду может заблокировать объект, и вы можете легко получить тупик или, по крайней мере, состязание по блокировке, которое можно найти, только взглянув как на код внутри класса, так и вызывающий код (и человек, пишущий один, может не иметь доступа к другому). По тем же причинам никогда не блокируйте Type.
3. По тем же причинам блокировка возможна только для частного участника. Таким образом, только код, внутренний для класса, может блокировать его, и вероятность конфликта блокировки ограничена этим местом.
4. Избегайте использования partial с таким кодом, и если вы все же используете partial, постарайтесь сохранить все блокировки на объекте в одном месте. Здесь нет технической причины, но это помогает, когда вам нужно подумать обо всех потенциальных местах, где может быть взята блокировка.
5. Никогда не блокируйте тип значения (целочисленный тип, структура и т. Д.). Это позволит привязать значение к новому объекту и заблокировать его. Затем, когда какой-то другой код попытается получить блокировку, он переключится на еще один объект и заблокирует его. По сути, нет никакой блокировки (за исключением теоретической оптимизации, бокса, использующего шаблон «легковес» (который он не делает), но на самом деле все будет еще хуже).
6. Иногда цель блокировки будет относиться к одному объекту, который используется, когда используется блокировка, и не используется, когда блокировка не используется. В этом случае использование этого объекта для блокировки помогает удобочитаемости кода, связывая блокировку с этим объектом.
7. Никогда не неправильно иметь объект, который существует исключительно для целей блокировки, поэтому, если вы не уверены, просто сделайте это.
8. Объект блокировки должен быть статическим, если какой-либо из затронутых объектов является статическим, и может быть экземпляром в противном случае. И наоборот, поскольку блокировка экземпляра более детализирована, чем статическая блокировка, экземпляр лучше, когда это применимо.

Answer 2

Да, он будет работать, поскольку _containers является статическим.

Возможно, вы захотите взглянуть и на ReaderWriterLockSlim , поэтому вам захочется так сильно заблокировать защиту (улучшение производительности)

Answer 3

Да, с _containers теоретически все в порядке, поскольку он имеет ту же область видимости, что и экземпляр словаря, то есть он является статическим (очевидно - это экземпляр словаря).

Однако однажды вас беспокоит общий дизайн - вы говорите, что он размещен на веб-сайте. Каждый рабочий asp.net - это отдельный процесс, поэтому ваш статический словарь может быть уничтожен, когда IIS перезапустит рабочий процесс из-за простоя или иным образом. Кроме того, у вас может быть несколько экземпляров словаря, если вы используете веб-сады (несколько сотрудников на приложение.)

Answer 4

Мне кажется довольно распространенным иметь явный экземпляр объекта, который используется исключительно для блокировки.

private readonly object containerLock = new object();
...
lock (containerLock) {
 ...
}

Другой совет: если вы работаете с .NET 4, рассмотрите возможность использования ConcurrentDictionary.