Примечание: первоначальный вариант этого поста описывал мое ошибочное предположение о том, что thread sanitizer выдал ложное срабатывание. Однако, ув.тов.Николай Меркин (кому-то он известен как Кодт с RSDN) указал на реальную ошибку. Поэтому текст был переработан.
Thread sanitizer выдал предупреждение на код, который я много лет считал корректным.
Для нетерпеливых вот самодостаточный пример на godbolt: https://godbolt.org/z/3xPadcnva.
Для всех остальных пояснение:
- на главной нити создается объект actual_repo. В этом объекте живут и std::mutex, и condition_variable (на котором будет осуществляться ожидание);
- ссылка на actual_repo передается в дочернюю нить. Через какое-то время дочерняя нить вызывает для actual_repo метод stop;
- главная же нить засыпает на вызове wait_for_stop у объекта actual_repo. Этот метод вернет управление только после того, как дочерняя нить вызовет stop;
- когда дочерняя нить вызывает stop, то главная нить просыпается, выходит из wait_for_stop, после чего разрушается объект actual_repo;
- после чего дожидаемся завершения дочерней нити и прекращаем работу.
Фокус здесь в том, что внутри stop условная переменная взводится (вызов notify_one()) без захвата мутекса.
А это ведет к тому, что главная нить может проснуться и уничтожить объект actual_repo еще до того, как дочерняя нить завершит вызов stop.
Т.е. деструктор для repo_basic::m_stop_initiated_cv может отработать еще до того, как на дочерней нити завершится вызов m_stop_initiated_cv.notify_one().
И как раз thread sanitizer и ругается на то, что в главной нити происходит модификация содержимого repo_basic::m_stop_initiated_cv тогда как на дочерней нити мы это содержимое только только прочитали.
Проблема же оказалась в том, что метод stop, вызванный на дочерней нити, не является атомарным. В нем сперва вызывается try_initiate_stop из базового класса. В этом самом try_initiate_stop захватывается mutex, меняется значение m_status, после чего mutex освобождается. Управление возвращается в метод stop и только после этого взводится m_stop_initiated_cv.
Именно эта неатомарность и является корнем зла.
Главная нить в методе wait_for_stop может захватить mutex и проверить m_status как раз в момент, когда на дочерней нити завершился try_initiate_stop, но еще не было обращения к m_stop_initiated_cv. И если такое произойдет, то главная нить уничтожит объект actual_repo еще до того, как на дочерней нити произойдет вызов m_stop_initiated_cv.notify_one().
А это приведет к тому, что notify_one() будет вызываться уже для убитого объекта.
Полагаю, на именно на эту проблему мне и указывал thread sanitizer, но я не смог с первого раза разобраться что же идет не так.
Так что в данном случае thread sanitizer выявил реальную проблему.
|
#include <thread> #include <mutex> #include <condition_variable> #include <chrono> #include <iostream> class repo_basic { protected: std::mutex m_lock; std::condition_variable m_stop_initiated_cv; int m_status{ 0 }; int try_initiate_stop() { std::lock_guard< std::mutex > lock{ m_lock }; if( !m_status ) { m_status = 1; return m_status; } return -1; } public: void wait_for_stop() { std::unique_lock< std::mutex > lock{ m_lock }; m_stop_initiated_cv.wait( lock, [this]{ return 0 != m_status; } ); } }; class actual_repo : protected repo_basic { public: using repo_basic::wait_for_stop; void stop() { const auto r = this->try_initiate_stop(); if( 1 == r ) { this->m_stop_initiated_cv.notify_one(); } } }; int main() { std::thread child_task; { actual_repo repo; child_task = std::thread{ [&repo]() { // Take some time to the main thread to call wait_for_stop. std::this_thread::sleep_for( std::chrono::milliseconds{ 250 } ); repo.stop(); } }; repo.wait_for_stop(); } child_task.join(); std::cout << "successful completion" << std::endl; } |
Комментариев нет:
Отправить комментарий