Грокаем C++ on Telegram: “Результаты ревью #опытным Спасибо всем коммент…”

Результаты ревью
#опытным

Спасибо всем комментаторам за актив и зоркие глаза! Да, да, ревью - это не только про знания, но еще и про умение замечать все мельчайшие недочеты. Но это лирика, погнали по проблемам. Напомню код:

#include <atomic>
#include <mutex>
#include <queue>
#include <thread>

struct Worker {
    Worker()
        : t([](std::stop_source st) {
              while (!st.stop_requested()) {
                  std::lock_guard<std::mutex> lock(m);
                  q.push(1);
              }
          }) {}

    std::thread t;
    std::mutex m;
    std::queue<int> q;
};

int main() {
    Worker w;
}

Начнем с простых и дойдем до серьезных:

🔞 Лишний хэдэр, соответственно лишнее время, которое тратится при компиляции на его анализ.

🔞 Внутри коллбэка используются поля класса, при этом захват у лямбды по умолчанию. Эта штука не заработает без захвата this.

🔞 Зачем-то в цикле идет захват лока, хотя никакой реальной конкурентной обработки очереди в данном конкретном случае нет. Его просто можно выкинуть.

🔞 Поток-то мы создали, а завершаться он как будет? std::thread всегда нужно мануально завершать. Но вообще говоря, токены останова сильно намекают на jthread'ы, поэтому можно их использовать и все заведется.

🔞 Коллбэк запуска потоков принимает std::stop_source. Для того, чтобы механизм остановки через стоп-сигналы из С++20 работал, коллбэку std::jthread нужно принимать std::stop_token.

🔞 Ну и основная "нетривиальная" проблема в этом коде. При создании потока в списке инициализации мы захватываем по сути еще неготовый объект. Мьютекс и очередь еще даже по умолчанию не инициализированы. И поток вполне может запуститься со ссылкой на эти неинициализированные данные. А это уже UB.

🔞 В ту же колоду отходит проблема при разрушении объекта. Даже, если мы сделаем jthread и он сможет хоть как-то разрушиться. Вызовы деструкторов полей класса происходят в порядке обратном объявлению. А значит очередь и мьютекс разрушаться до вызова деструктора потока. Что опять же приводит к dangling reference и ub.
Пофиксить просто - объявляем поток в самом низу и дело в шляпе.

Вот исправленная версия:

#include <queue>
#include <thread>
struct Worker {
    Worker()
        : t([this](std::stop_token st) {
              while (!st.stop_requested()) {
                  q.push(1);
              }
          }) {}
    std::queue<int> q;
    std::jthread t;
};
int main() {
    Worker w;
}

Спасибо, @vm05s24 и @thonease за пристальное ревью)

Fix your flaws. Stay cool.