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 тогда как на дочерней нити мы это содержимое только только прочитали.
Полный код примера под катом. Если раскомментировать строчку в repo_basic::try_initiate_stop, то thread sanitizer будет доволен и ничего не диагностирует.
Собственно, у меня два (возможно, риторических) вопроса к аудитории:
- это ложное срабатывание thread sanitizer-а или же в таком коде реально есть проблема гонок? Я лично всегда думал, что здесь гонок нет. Но есть вероятность, что ошибаюсь. А так как ошибаюсь часто, то уже сомневаюсь даже в том, в чем раньше был уверен;
- если это таки ложное срабатывание, то как тогда быть с кодом, чтобы его можно было проверять thread sanitizer-ом, но не натыкаться на это ложное срабатывание снова и снова?
В качестве дополнения: в примере максимально выхолощенная версия, выделенная из реального кода. Поэтому там есть и repo_basic, и другие усложнения. Полагаю, что для демонстрации данного случая можно было и более простой код написать. Однако, оригинальный код специально написан с учетом того, что после метода stop к actual_repo обращаться нельзя (т.е. уже учтено, что actual_repo может быть разрушен параллельно работающей нитью). Да и сам метод stop в actual_repo специально написан так, что последнее, что он делает -- это обращается к m_stop_initiated_cv.notify_one().