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

The analyzer has detected a possible error in the code: several threads change a shared resource without synchronization.

Let's look at the example:

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;
}

The 'GetNamesById' method returns names based on the list of identifiers. The 'Parallel.ForEach' method processes all the elements of the 'ids' collection for this purpose. The method creates and executes an SQL query for each element.

The problem is that the captured local variable 'query' is a shared resource of threads executing in 'Parallel.ForEach'. Different threads will access the same object asynchronously. This may result in incorrect program behavior.

Below is a description of a possible error:

In the first thread, an SQL query with an 'id' equal to 42 is written to the 'query' variable. This value should then be passed to 'ProcessQuery'.
In the second thread, a new SQL query with an 'id' equal to 12 is written to 'query'.
Both threads call 'ProcessQuery' using the 'query' value with 'id' equal to 12.
As a result, 'ProcessQuery' is called twice with the same value. In this case, the value assigned in the first thread is lost.

The correct method implementation may look like this:

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;
}

Here, each thread handles its own 'query' variable. This code causes no issues since threads do not share resources.

Look at another example:

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

The 'CountFails' method counts the exceptions when executing operations on the 'ids' collection elements. There is also an unsynchronized access to a shared resource in this code. The increment and decrement operations are not atomic, so the correct exception counting is not guaranteed in this case.

The correct method implementation may look like this:

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

The 'Interlocked.Increment' method is used for correct counting. The method increments a variable atomically.

This diagnostic is classified as:

CWE-821