ASP.NET/Статический класс Race Condition?

Question

У меня есть приложение ASP.NET с большим количеством динамического содержимого. Содержание одинаково для всех пользователей, принадлежащих конкретному клиенту. Чтобы уменьшить количество обращений к базе данных, необходимых для запроса, я решил кэшировать данные на уровне клиента. Я создал статический класс («ClientCache») для хранения данных.
Наиболее часто используемый метод класса - это, безусловно, «GetClientData», который возвращает объект ClientData, содержащий все сохраненные данные для конкретного клиента. ClientData загружается лениво, хотя: если запрошенные данные клиента уже кэшированы, вызывающая сторона получает кэшированные данные; в противном случае данные извлекаются, добавляются в кэш, а затем возвращаются вызывающей стороне.

В конце концов я начал получать периодические сбои в методе GetClientData в строке, где объект ClientData добавляется в кэш. Вот тело метода:

public static ClientData GetClientData(Guid fk_client)
{
    if (_clients == null)
        _clients = new Dictionary<Guid, ClientData>();

    ClientData client;
    if (_clients.ContainsKey(fk_client))
    {
        client = _clients[fk_client];
    }
    else
    {
        client = new ClientData(fk_client);
        _clients.Add(fk_client, client);
    }
    return client;
}

Текст исключения всегда выглядит как «Объект с таким же ключом уже существует». Конечно, я пытался написать код, чтобы просто не было возможности добавить клиента в кеш, если он уже существует.

На данный момент, я подозреваю, что у меня есть условие гонки, и метод выполняется дважды одновременно, что может объяснить, как происходит сбой кода. Что меня смущает, так это то, как метод может быть выполнен дважды одновременно. Насколько я знаю, любое приложение ASP.NET когда-либо выставляет только один запрос за раз (поэтому мы можем использовать HttpContext.Current).

Итак, является ли эта ошибка вероятной причиной гонки, которая потребует установки блокировок в критических секциях? Или я упускаю более очевидную ошибку?

Answer 1

Если приложение ASP.NET обрабатывает только один запрос за раз, все сайты ASP.NET будут иметь серьезные проблемы. ASP.NET может обрабатывать десятки одновременно (обычно 25 на ядро ​​процессора).

Вы должны использовать ASP.NET Cache вместо использования собственного словаря для хранения вашего объекта. Операции с кешем поточнобезопасны.

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

Редактировать

Комментарий к этому ответу гласит: -

Только атомарные операции в кеше поточно-ориентированы. Если вы делаете что-то вроде проверки
если ключ существует, а затем добавить его, это НЕ является потокобезопасным и может привести к тому, что элемент
перезаписаны.

Стоит отметить, что если мы чувствуем, что нам нужно сделать такую ​​операцию атомарной, то, вероятно, кеш - не то место для ресурса.

У меня довольно много кода, который делает именно так, как описано в комментарии. Однако сохраняемый ресурс будет одинаковым в обоих местах. Следовательно, если существующий элемент в редких случаях перезаписывается, единственной ценой является то, что один поток излишне генерирует ресурс. Стоимость этого редкого события намного меньше стоимости попытки сделать операцию атомарной каждый раз, когда делается попытка получить к ней доступ.

Answer 2

Это очень легко исправить:

private _clientsLock = new Object();

public static ClientData GetClientData(Guid fk_client)
{
  if (_clients == null)
    lock (_clientsLock)
      // Check again because another thread could have created a new 
      // dictionary in-between the lock and this check
      if (_clients == null) 
        _clients = new Dictionary<Guid, ClientData>();

  if (_clients.ContainsKey(fk_client))
    // Don't need a lock here UNLESS there are also deletes. If there are
    // deletes, then a lock like the one below (in the else) is necessary
    return _clients[fk_client];
  else
  {
    ClientData client = new ClientData(fk_client);

    lock (_clientsLock)
      // Again, check again because another thread could have added this
      // this ClientData between the last ContainsKey check and this add
      if (!clients.ContainsKey(fk_client))
       _clients.Add(fk_client, client);

    return client;
  }
}

Имейте в виду, что всякий раз, когда вы возитесь со статическими классами, у вас могут возникнуть проблемы с синхронизацией потоков. Если есть какой-то статический список уровня класса (в данном случае _clients, объект Dictionary), то DEFINITELY будут проблемы с синхронизацией потоков для решения.

Answer 3

вам нужен потокобезопасный и минимизировать блокировку.
см. Двойная проверка блокировки (http://en.wikipedia.org/wiki/Double-checked_locking)

пишите просто с TryGetValue.

public static object lockClientsSingleton = new object();

public static ClientData GetClientData(Guid fk_client)
{
    if (_clients == null) {
        lock( lockClientsSingleton ) {
            if( _clients==null ) {
                _clients = new Dictionary``();
            }
        }
    }
    ClientData client;
    if( !_clients.TryGetValue( fk_client, out client ) )
    {
        lock(_clients) 
        {
            if( !_clients.TryGetValue( fk_client, out client ) ) 
            {
                client = new ClientData(fk_client)
                _clients.Add( fk_client, client );
            }
        }
    }
    return client;
}

Answer 4

Ваш код действительно предполагает, что в функции одновременно входит только один поток.

Это просто не будет верно в ASP.NET

Если вы настаиваете на том, чтобы делать это таким образом, используйте статический семафор для блокировки области вокруг этого класса.