[prog.c++] Еще один любопытный баг на стыке многопоточности и ООП

Появился повод рассказать об еще одной гримасе многопоточности: на днях был найден и исправлен любопытный баг, в котором вроде бы корректная работа с многопоточностью наложилась на особенности реализации ООП.

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

class data_owner_t { public:    virtual void update() = 0;    ... }; class data_repository_t {    std::mutex lock_;    some_container_t<data_owner_t *> owners_;    ... public:    void add(data_owner_t & owner) {       std::lock_guard lock{lock_};       owners_.insert(&owner);    }    void remove(data_owner_t & owner) {       std::lock_guard lock{lock_};       owners_.erase(&owner);    }    void update_all() {       std::lock_guard lock{lock_};       for(auto * p : owners_)          p->update();    }    ... };

Виртуальный метод здесь всего один -- это data_owner_t::update. Вызывается он только внутри data_repository_t::update_all, в цикле, перебирающем всех зарегистрированных owner-ов. Значит в какой-то момент времени внутри data_repository_t оказывается невалидный указатель на owner-а. Но как и почему?

Практически сразу же стало понятно, что проблема возникает если пересекается изъятие owner-а и итерация по списку owner-ов. Ощущение было такое, что метод data_repository_t::remove успешно отрабатывает в тоже самое время, пока работает data_repository_t::update_all. Поэтому в цикле внутри update_all мы время от времени натыкаемся на уже переставший быть актуальным указатель.

Это была бы хорошая версия, но она вела в настоящий тупик: было совершенно непонятно, как работа remove и update_all могла пересечься, если они обе защищены одним и тем же мутексом. Тем не менее, было очевидно, что в списке owner-ов оказывается невалидный указатель. Т.е. происходило то, что вроде как не должно было происходить. Но происходило.

А дело оказалось вот в чем.

Часть наследников data_owner_t выполняли добавление самих себя в репозиторий в конструкторе, а изъятие в деструкторе. Чтобы не повторять одно и тоже снова и снова, был сделан вспомогательный класс по типу вот такого:

class autostarted_data_owner_t : public data_owner_t {    data_repository_t & repo_; public:    autostarted_data_owner_t(data_repository_t & repo) : repo_{repo} {       repo_.add(*this);    }    ~autostarted_data_owner_t() override {       repo_.remove(*this);    } };

Только конструктор и деструктор. Все остальные методы, унаследованные от data_owner_t оставались в точности такими же, как и в базовом классе. В том числе унаследованный update оставался чистым виртуальным методом.

Далее реальные классы owner-ов наследовались от этого autostared_data_owner_t. Поэтому экземпляры реальных owner-ов автоматически добавлялись в репозиторий при создании и изымались из репозитория при удалении. И проблема была как раз в автоматическом изъятии при удалении.

Когда реальный owner удаляется, то сперва вызывается его собственный деструктор. Потом начинают вызваться деструкторы базовых классов. И в какой-то момент времени мы оказывается в деструкторе autostarted_data_owner_t. Здесь мы делаем вызов data_repository_t::remove. И, если в данный момент работает data_repository_t::update_all, то мы ждем завершения update_all. Тут все правильно, указатель на еще существующий объект все еще лежит внутри data_repository.

Но фокус в том, на что именно он указывает? А указывает он не на полноценный объект реального owner-а, а всего лишь на тот "огрызок", который соответствует autostarted_data_owner_t. И у этого огрызка нет реализации метода update. Для него update является чистым виртуальным методом.

Дело в том, что когда завершается деструктор производного класса и вызывается деструктор базового класса, содержимое VMT обновляется так, чтобы элементы в ней указывали на реализации виртуальных методов именно из базового класса, а не из производного.

Так вот, при определенном стечении обстоятельств складывается ситуация, когда хранящийся в data_repository указатель, который раньше указывал на полноценного owner-а, вдруг начинает указывать на огрызок owner-а, соответствующий типу autostarted_data_owner_t. Соответственно, если вызвать метод update в данный момент, то как раз и произойдет вызов чистого виртуального метода. На что, собственно, и довелось наступить :)

Тоже самое, но в обратном порядке, происходит и при конструировании объекта: когда работает конструктор базового класса, то VMT содержит указатели на реализации виртуальных методов из базового класса, при переходе к конструктору производного класса VMT обновляется и начинает указывать уже на реализации виртуальных методов из производного класса). Поэтому такой же баг мог возникнуть не только при пересечении remove и update_all, но и при пересечении add и update_all.

Так что еще раз повторю то, что говорил неоднократно: многопоточнось -- это пот, боль и кровь. Фокусы можно ожидать на каждом шагу. Даже там, где, казалось бы, все корректно закрыто mutex-ами. Поэтому если у вас есть возможность не писать многопоточный код, то не пишите его.