25.04.2022 15:11
foto_shooter 0 2800 Источник

0940_AkkaNET_Error_ru/image1.png


"Статический анализ нужно использовать регулярно, а не только перед релизами… Чем раньше найдена ошибка, тем дешевле её исправление..." – вы уже слышали это 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, если интересует подобный контент.

P.S. Если хотите поделиться статьёй с англоязычной аудиторией, вот ссылка на англ. версию.

Комментарии (0)