четверг, 21 марта 2019 г.

[prog.bugs] Интересная ошибка, связанная с многопоточностью

В минувший вторник убил целый рабочий день на разбирательство с любопытным багом. В многопоточном коде, в котором пришлось иметь дело с голыми std::mutex-ами и std::thread. Кому интересно, милости прошу под кат. Ошибка, в общем-то, имеет C++ную специфику, но, полагаю, во что-то подобное можно втоптаться и в любом другом языке с ручным управлением ресурсами.

Итак, суть в том, что в один прекрасный момент тест, который до этого долгое время работал исправно, начал стабильно падать. Причем падал по разному под VC++ и MinGW, что добавило ярких красок в процесс поиска причины проблемы. В итоге длительного выкуривания бамбука и множества экспериментов причина была найдена. Ниже я попытаюсь в максимально упрощенном виде рассказать что происходило и почему это происходило.

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

Т.е. у нас есть экземпляр work_thread_t. Когда этот экземпляр понимает, что он больше не нужен, он отсылает указатель на самого себя экземпляру terminator_thread_t. Когда terminator_thread_t получает указатель на work_thread_t, то выполняется очистка всякого-разного, после чего ставший ненужным work_thread_t уничтожается.

Схематично work_thread_t может быть представлен в следующем виде:

class work_thread_t : public std::enable_shared_from_this<work_thread>
{
   // Очередь заявок для terminator_thread.
   msg_queue_handle_t terminator_queue_;
   ...
   // Рабочая нить.
   std::thread thread_;
   // Собственная очередь заявок.
   msg_queue_handle_t self_queue_;
   ...
public:
   work_thread_t(...) {
      ...
      // В конструкторе рабочая нить запускается.
      thread_ = std::thread{ [this] { body(); } };
   }

   ~work_thread_t() {
      // Если рабочая нить еще не завершилась, то ждем этого события.
      if(thread_.joinable())
         thread_.join(); // (1) Запоминаем эту строчку.
   }

private:
   void body() {
      while(...) { // Читаем из self_queue_ пока не закроют.
         ...; // Обрабатываем прочитанное.
      }

      // Если оказались здесь, значит self_queue_ уже закрыта.
      // Терминатор должен прекратить нашу жизнь.
      auto self_shptr = shared_from_this();
      ... // Какие-то действия с self_shptr.
      terminator_queue_.push<std::shared_ptr<work_thread>>(std::move(self_shptr));
   }
};

Проблема проявлялась в том, что временами вызов thread_.join() выполнялся на контексте самой рабочей нити. Т.е. деструктор work_thread_t вызывался на самой рабочей нити. Тогда как по задумке (и в большинстве случаев на практике) разрушение work_thread_t и обращение к thread_.join() должно было происходить на контексте нити-терминатора.

Ключевой момент в реализации work_thread_t в том, что он наследуется от enable_shared_from_this. К моменту, когда у work_thread_t закрывают очередь сообщений актуальные shared_ptr<work_thread_t> остаются только где-то в потрохах структур данных, доступ к которым имеет нить-терминатор. И как раз одна из задач нити-терминатора заключается в том, чтобы удалить все эти оставшиеся shared_ptr<work_thread_t>. Поэтому основной рабочий цикл нити-терминатора схематично можно представить так:

void terminator_thread_t::body() {
   while(has_messages == self_queue_.wait()) {
      auto worker = self_queue_.pop<std::shared_ptr<work_thread>>();
      ...; // Удаление всего, что связано с worker-ом.

      // К этому моменту worker -- это единственный живой shared_ptr,
      // ссылающийся на worker-а.
      // Поэтому мы прекращаем жизнь worker-а.
      worker.reset();
   }
   ...
}

Т.е. нить-терминатор должна была владеть последним экземпляром shared_ptr. Но регулярно ломающийся тест показывал, что это не так. Что означало, что где-то еще оставался какой-то shared_ptr. Причем "где-то" оказалось довольно таки ограниченным понятием: рабочая нить где-то у себя оставляла этот самый shared_ptr. Оставалось найти где.

Проблема оказалась в реализации push для очереди сообщений. Т.к. эта самая push состоит из двух частей: собственно верхнеуровневый шаблонный push, который виден программисту, и низкоуровневый enqueue, который и заталкивает объект-сообщение в очередь.

Разделение на push и enqueue потребовалось потому, что enqueue работает только с наследниками специального типа message_t. Тогда как push -- эта шаблонная функция, которая конструирует нужный экземпляр message_t в зависимости от типа параметров:

template<typename T, typename... Args>
void push(Args &&... args) {
   enqueue(std::make_shared<message_t>(... /* Тут какая-то магия */));
}

Для случая, когда в качестве T выступает некий произвольный тип, не наследник от message_t (как в данном случае, где в качестве T выступает shared_ptr<work_thread_t>), создается специальный экземпляр, в конструктор которого передаются все аргументы push-а:

template<typename T>
struct special_message_t : public message_t {
   T payload_;

   template<typename... Args>
   special_message_t(Args &&... args) : payload_{std::forward<Args>(args)...} {}
};

Т.е. в моем случае push превращался во что-то вроде:

template<>
void push<std::shared_ptr<work_thread_t>, std::shared_ptr<work_thread_t>>(
   std::shared_ptr<work_thread_t> && arg)
{
   enqueue(
         std::make_shared<special_message_t<shared_ptr<worker_thread_t>>>(
               std::forward<std::shared_ptr<work_thread_t>&&>(arg)
         )
   );
}

Можно обратить внимание, что злополучный экземпляр shared_ptr<work_thread_t> везде передается как rvalue-reference, т.к. он должен был переместиться внутрь special_message_t. И, соответственно, должен был навсегда уйти с контекста текущей рабочей нити на контекст нити-терминатора. Но почему-то не уходил... :(

Собака же оказалась зарыта в прототипе метода enqueue, который принимал свой аргумент по константной ссылке:

void enqueue(const std::shared_ptr<message_t> &);

Т.е. время жизни объекта special_message_t<shared_ptr<work_thread_t>> продлевалось до конца вызова enqueue!

В большинстве случаев enqueue завершался до того, как нить-терминатор успевала извлечь сообщение из своей очереди. Но иногда, один раз на несколько сотен, нить-терминатор не просто успевала извлечь сообщение из очереди, но и успевала обработать извлеченное сообщение и забыть про этот экземпляр. В этих случаях special_message_t<shared_ptr<work_thread_t>> продолжал жить пока на контексте рабочей нити оставался shared_ptr на special_message_t. А значит и оставался еще один shared_ptr на work_thread_t. Когда enqueue звершал свою работу эти shared_ptr тут же разрушались и вызывался деструктор у work_thread_t. Но вызывался деструктор на контексте самой рабочей нити. А не на контексте нити-терминатора.

Почему enqueue получал аргумент по константной ссылке, а не по значению, сейчас уже доподлинно не выяснишь. Это решение принималось много лет назад в совсем других условиях. Сейчас же это решение сыграло злую шутку.


Этот баг мне понравился еще и тем, что он показывает, как на грабли в многопоточном коде можно наступить даже там, где, казалось бы, все было продумано. Тут ведь нет ни гонок (в привычном нам понимании), ни дедлоков. Ситема владения экземплярами work_thread_t была выверена и, действительно, принципиальная схема передачи "последнего" shared_ptr-а от рабочей нити к нити-терминатора была корректной. В дело вмешалась незамеченная деталь реализации операции постановки сообщения в очередь.


Disclaimer. В посте описана грубая и максимально упрощенная схема. В реальности все было гораздо сложнее и запутаннее. Кроме того в посте используются shared_ptr, т.к. этот тип сейчас должен быть хорошо знаком современным C++ разработчикам. Но в реальном коде кое-где вместо shared_ptr использовались свои велосипеды для интрузивных умных указателей.

Upd. Для того, чтобы можно было себе представить, что же происходило в реальности, немного дополню.

Итак, есть кооперация, которая владаеет агентам и биндерами для агентов, из биндеров есть ссылки на диспетчеров, а оттуда уже на рабочие нити. Когда кооперация разрушается, то должны разрушиться все биндеры. При разрушении биндеров могут быть разрушен какой-то диспетчер, который больше никем не используется. В этом случае так же должны быть завершены все рабочие нити этого диспетчера (т.е. для них должен быть вызван join()).

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

Рано или поздно "умирает" последний агент (на самом деле физически он не умирает, но работать перестает). И вот когда этот последний агент, обрабатывая свою последнюю заявку на своем рабочем контексте, говорит кооперации, что большее у нее никого в живых не осталось, вот тогда приходит время физически разрушить кооперацию.

Для этого shared_ptr на кооперацию передается нити-терминатору. И это должен оказаться последний shared_ptr. Этот последний shared_ptr умрет здесь, на нити-терминатора и нить-терминатор разрушает все внутренности кооперации. Т.е. физически уничтожаются агенты и физически уничтожаются биндеры. При уничтожении биндеров могут быть уничтожены рабочие нити. Следовательно, для рабочих нитей нужно сделать join().

Поэтому и есть нить-терминатор, которой передается информация об умирающей кооперации. Эта нить отдельная, она совершенно спокойно может делать join() для всех остальных нитей. Т.е. физически разрушая кооперацию, агентов, биндеры и диспетчеры на нити-терминаторе можно не боятся о том, что join() для какой-либо из нитей уничтожаемых диспетчеров будет вызван не на том контексте.

Фактически в баге, с которым я столкнулся, слишком долго прожил shared_ptr на кооперацию. Получилось, что физически кооперация была уничтожена не на нити-терминаторе, а на одной из рабочих нитей (на которой как раз дорабатывал последний агент кооперации). И так же получилось, что внутри кооперации оказался и умный указатель на эту рабочую нить. Поэтому и вышло так, что нить попробовала уничтожить саму себя.

4 комментария:

Сергей Скороходов комментирует...

Исключительно круто, спасибо!

Yauheni Akhotnikau комментирует...

@Сергей Скороходов
Рад, что понравилось.

Unknown комментирует...

Ну так че, какое решение ты выбрал. Стал push'ить голый указатель, в потоке-терминаторе сделал sleep() пока счетчик shared_ptr не объединиться (не станет == 1) или в деструкторе стал проверять id потока и делать detach() в рабочем потоке вместо join() (если владение разделяется, то и удаление должно быть совместным)?

Yauheni Akhotnikau комментирует...

@Unknown:
Стал отсылать shared_ptr не в чистом виде, а обернутым в простую структуру. В нити-терминаторе содержимое shared_ptr муваю в локальную переменную. Т.о. мне оказывается неважно, сколько времени проживет моя вспомогательная структура.