V3190. Concurrent modification of a variable may lead to errors.

Анализатор обнаружил возможную ошибку в коде: несколько потоков без синхронизации изменяют общий ресурс.

Рассмотрим пример:

ConcurrentBag<String> GetNamesById(List<String> ids)
{
  String query;
  ConcurrentBag<String> result = new();
  Parallel.ForEach(ids, id =>
  {
    query = $@"SELECT Name FROM data WHERE id = {id}";
    result.Add(ProcessQuery(query));
  });
  return result;
}

Метод 'GetNamesById' возвращает имена в соответствии с переданным списком идентификаторов. Для этого методом 'Parallel.ForEach' обрабатываются все элементы коллекции 'ids': для каждого из них составляется и исполняется SQL-запрос.

Проблема в том, что захваченная локальная переменная 'query' является общим разделяемым ресурсом потоков, исполняющихся в 'Parallel.ForEach'. Разные потоки будут производить несинхронизированный доступ к одному объекту. Это может привести к некорректному поведению программы.

Ниже приведено описание возможной проблемной ситуации:

В первом потоке в переменную 'query' записывается SQL-запрос с 'id' равным 42. Это значение должно далее передаваться в 'ProcessQuery'.
Во втором потоке в 'query' записывается новый SQL-запрос с 'id' равным 12.
Оба потока вызывают 'ProcessQuery', используя значение 'query' с 'id' равным 12.
В результате 'ProcessQuery' вызван дважды с одним и тем же значением. При этом теряется значение, полученное при присваивании в первом потоке.

Корректная реализация метода может выглядеть следующим образом:

ConcurrentBag<String> GetNamesById(List<String> ids)
{
  ConcurrentBag<String> result = new();
  Parallel.ForEach(ids, id =>
  {
    String query = $@"SELECT Name FROM data WHERE id = {id}";
    result.Add(ProcessQuery(query));
  });
  return result;
}

Здесь каждый поток работает с собственной переменной 'query'. В таком случае проблем не будет, так как нет разделяемого между потоками ресурса.

Рассмотрим ещё один пример:

int CountFails(List<int> ids)
{
  int count = 0;
  Parallel.ForEach(ids, id =>
  {
    try
    {
      DoSomeWork(id);
    }
    catch (Exception ex)
    {
      count++;
    }
  });
  return count;
}

Метод 'CountFails' считает количество исключений при выполнении операций над элементами коллекции 'ids'. Этот код также содержит проблему несинхронизированного доступа к общему ресурсу. Операции инкремента и декремента не являются атомарными, поэтому корректный подсчёт количества исключений в этом случае не гарантирован.

Корректная реализация метода может выглядеть следующим образом:

int CountFails(List<int> ids)
{
  int count = 0;
  Parallel.ForEach(ids, id =>
  {
    try
    {
      DoSomeWork(id);
    }
    catch (Exception ex)
    {
      Interlocked.Increment(ref count);
    }
  });
  return count;
}

Здесь для корректного подсчета используется метод 'Interlocked.Increment', предоставляющий атомарную операцию инкремента переменной.

Данная диагностика классифицируется как:

CWE-821