В минувший вторник убил целый рабочий день на разбирательство с любопытным багом. В многопоточном коде, в котором пришлось иметь дело с голыми 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 на кооперацию. Получилось, что физически кооперация была уничтожена не на нити-терминаторе, а на одной из рабочих нитей (на которой как раз дорабатывал последний агент кооперации). И так же получилось, что внутри кооперации оказался и умный указатель на эту рабочую нить. Поэтому и вышло так, что нить попробовала уничтожить саму себя.
Исключительно круто, спасибо!
ОтветитьУдалить@Сергей Скороходов
ОтветитьУдалитьРад, что понравилось.
Ну так че, какое решение ты выбрал. Стал push'ить голый указатель, в потоке-терминаторе сделал sleep() пока счетчик shared_ptr не объединиться (не станет == 1) или в деструкторе стал проверять id потока и делать detach() в рабочем потоке вместо join() (если владение разделяется, то и удаление должно быть совместным)?
ОтветитьУдалить@Unknown:
ОтветитьУдалитьСтал отсылать shared_ptr не в чистом виде, а обернутым в простую структуру. В нити-терминаторе содержимое shared_ptr муваю в локальную переменную. Т.о. мне оказывается неважно, сколько времени проживет моя вспомогательная структура.