Сергей Васильев

25 Апр 2022

Теги:

#CSharp

Зачем нужен статический анализ? Разбираем на примере ошибки из Akka.NET

"Статический анализ нужно использовать регулярно, а не только перед релизами... Чем раньше найдена ошибка, тем дешевле её исправление..." – вы уже слышали это 100 раз. Сегодня ещё раз ответим на вопрос "зачем?". Поможет нам ошибка из проекта Akka.NET.

Ошибка

Начнём с задания. Нужно найти дефект в этом фрагменте кода:

protected override bool ReceiveRecover(object message)
{
  switch (message)
  {
    case ShardId shardId:
      _shards.Add(shardId);
      return true;
    case SnapshotOffer offer when (offer.Snapshot is 
                                   ShardCoordinator.CoordinatorState state):
      _shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards));
      return true;
    case SnapshotOffer offer when (offer.Snapshot is State state):
      _shards.Union(state.Shards);
      _writtenMarker = state.WrittenMigrationMarker;
      return true;
    case RecoveryCompleted _:
      Log.Debug("Recovery complete. Current shards [{0}]. Written Marker {1}", 
                string.Join(", ", _shards), 
                _writtenMarker);

      if (!_writtenMarker)
      {
        Persist(MigrationMarker.Instance, _ =>
        {
          Log.Debug("Written migration marker");
          _writtenMarker = true;
        });
      }
      return true;
    case MigrationMarker _:
      _writtenMarker = true;
      return true;
  }
  ....
}

Разберём, в чём тут дело.

Тип переменной _shards – HashSet<ShardId>. В приведённом выше коде вызываются несколько методов, которые изменяют состояние этого множества.

HashSet<T>.Add:

_shards.Add(shardId);

HashSet<T>.UnionWith:

_shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards));

Однако один из вызовов – неправильный:

_shards.Union(state.Shards);

Он не меняет состояние объекта _shards. Enumerable.Union – метод-расширения из LINQ, который возвращает изменённую коллекцию, а не меняет исходную. Значит, результат вызова метода должен быть или куда-то сохранён, или как-то использован. Этого в коде тоже нет.

Анализатор PVS-Studio выдал такое предупреждение: V3010 The return value of function 'Union' is required to be utilized. Akka.Cluster.Sharding EventSourcedRememberEntitiesCoordinatorStore.cs 123

Исправленный код, кстати, выглядит так:

_shards.UnionWith(state.Shards);

Как мы нашли ошибку или в 101-ый раз о пользе статического анализа

У нас на сервере каждую ночь запускается анализ нескольких Open Source проектов. В их числе – Akka.NET. Это помогает:

• дополнительно тестировать анализатор;
• рассказывать о пользе стат. анализа в подобных заметках.

Подробнее про систему писали здесь.

А теперь немного хронологии появления и исправления проблемы.

20.04.2022:

• в dev-ветку проекта Akka.NET попадает код с ошибкой (ссылка на конкретную строку);

21.04.2022:

• код анализируется у нас на сервере, и мне приходит письмо с информацией о предупреждениях;
• я изучаю проблему и открываю issue на GitHub;
• разработчики исправляют ошибку. Ссылка на коммит.

Считаю, что отработали слаженно и оперативно. Разработчикам уважение за быстрый фикс.

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

Что делать?

• Использовать статический анализатор. Загрузить можно здесь. Если применить промокод pvs_akka, триал будет работать 30 дней, а не 7.
• Подписаться на меня в Twitter, если интересует подобный контент.

#CSharp

0

0

0

0

ПОДЕЛИТЬСЯ

Теги:

#CSharp

Присылаем лучшие статьи раз в месяц