Комментарии 36
Уточните, пожалуйста, почему вы решили что в потоке безопасно проверять состояние указателя, не завернув проверку в какой-нибудь мьютекс? Насколько я помню, weak/shared указатели не являются потокобезопасными.
All member functions (including copy constructor and copy assignment) can be called by multiple threads on different instances of shared_ptr without additional synchronization even if these instances are copies and share ownership of the same object. If multiple threads of execution access the same shared_ptr without synchronization and any of those accesses uses a non-const member function of shared_ptr then a data race will occur; the shared_ptr overloads of atomic functions can be used to prevent the data race.
запускает лямбда-функцию на выполнение в отдельном потоке
пытается взять сильную ссылку на экземпляр класса с проверкой результата; проверка проходит успешно
уничтожает единственную внешнюю сильную ссылку
Почему здесь происходит гонка данных? Это связано с тем, что weak_ptr::lock() не thread-safe?
Просивший застолбил валидность объекта. Гарантировал ему срок жизни.
Такую проверку не следует использовать для определения отсутствия/наличия внешних сильных ссылок, а в коде она имеет именно такой смысл.
Но гонку Вы понимаете слишком узко — только как обращение к уничтоженному объекту.
В данном случае от результата гонки зависит приход или неприход сообщения в валидный listener после уничтожения объекта зомби бизнес-логикой.
Звучит не очень страшно, но только на первый взляд.
thread1:
sp->work();
sp->release();
std::cout << "Here sp released";
thread2:
sp = wp.lock();
std::cout << "Log \"Here sp released\" not printed";
Тогда здесь в строках std::cout действительно будет то, что Вы подразумеваете под «гонкой»
sp->release();
Как я понимаю, это явная команда остановки, а значит — отказ от преимуществ RAII. Очень важно, что эта команда обрабатывается не shared_ptr, а объектом, находящимся в нём.
Посмотрите, как устроены main() в моих примерах — там просто выход стековой переменной из scope.
По определению, состояние гонки — это когда результат выполнения программы может существенно различаться в зависимости от взаимного порядка выполнения операций в потоках. В чём именно это различие проявится — это уже подробности.
Небольшое замечание: std::cout сам по себе непотокобезопасен.
2. Дополнительных копий бизнес-логика не делала, так что вполне могла ожидать уничтожения объекта.
Я конечно над этим не задумывался, но мне кажется, что RAII — это ответственность именно конкретного участка кода, не нужно надеяться на raii извне.
То есть, когда Вы обмениваетесь умными указателями, то эти указатели — это не raii, это всеголишь гарантия, что не будет утечек памяти. А raii должен обеспечивать каждый сам
И проблема в пользователях, которые почему-то решают, что закрытие вкладки в браузере должно приводить к остановке проигрывания музыки из ролика, который в этой вкладке крутился.
RAII — вполне себе жизнеспособная идиома, особенно в условиях наличия в языке механизма исключений, которые могут неожиданно прервать выполнение произвольной функции в произвольный момент, в том числе между явным выделением ресурса и его явным освобождением.
Что такое «RAII извне» — вообще не понял.
Умные указатели могут не только памятью рулить, но и другими типами ресурсов — например потоками, как в моих примерах.
И отсутствие утечек этих ресурсов они не совсем гарантируют.
Можно так, да.
Но это не значит, что верхний уровень сможет таким образом скомпенсировать любой бардак в служебном классе. Требования к служебному классу всё равно придётся предъявлять — например, чтобы команда явной остановки не блокировала выполнение на неопределённое время.
А раз требования всё равно есть — то почему бы не потребовать такой служебный класс, который можно просто создавать и уничтожать, подразумевая под уничтожением остановку? Просто потому, что таким классом удобнее пользоваться.
std::cout сам по себе непотокобезопасен
Unless sync_with_stdio(false) has been issued, it is safe to concurrently access these objects from multiple threads for both formatted and unformatted output.
Но этого не достаточно. Надо ещё как минимум не перемешать вывод из разных потоков. А убедиться, что вывод реально перемешивается, очень даже вполне просто.
Так что не путайте тёплое с мягким.
Однажды видел софтину, которая при загруженном двухъядерном проце выводила в консоль прям конкретно посимвольную зебру из двух потоков.
Надо ещё как минимум не перемешать вывод из разных потоков.В пределах одной записи (<<) вывод и не перемешивается, а уж как именно упорядочивать отдельные записи можете знать только вы.
выводила в консоль прям конкретно посимвольную зебру из двух потоковЗначит писали посимвольно.
#pragma once
#include <ostream>
namespace utility {
class Data
{
public:
Data(int a, int b, int c)
: _a(a)
, _b(b)
, _c(c)
{}
private:
int _a;
int _b;
int _c;
friend std::ostream& operator<<(std::ostream& os, const Data& data);
};
std::ostream& operator<<(std::ostream& os, const Data& data);
} // namespace utility
#include <iostream>
#include <vector>
#include <future>
#include "utility.h"
int main()
{
const size_t size = 500;
auto data = std::vector<utility::Data>();
data.reserve(size);
for (size_t i = 0; i < size; ++i) {
data.emplace_back(i, i * 2, i * 3);
}
auto fn = [&data;](){
for (auto const& elem : data) {
std::cout << elem;
}
};
auto future1 = std::async(std::launch::async, fn);
auto future2 = std::async(std::launch::async, fn);
return 0;
}
Какого поведения следует от него ожидать?
Так все проблемы не в умных указателях, а в том, что есть несколько способов сделать detached thread.
Один способ — через ссылку изнутри потока на собственного владельца, а другой — через явный вызов detach (и тогда система будет держать свою собственную ссылку на поток, хоть и не на объект-фасад).
И потом ходить говорить "так и знал, кривые кирпичи мне подсунули".
Вот эти рассуждения "я прибил сессию, а она всё равно что-то успела поделать" — это потому, что всё так задумано. Точнее, не задумана атомарность и/или транзакционность.
Вообще, зомби делаются на раз безо всяких циклических ссылок:
struct ThreadData { ..... };
int main() {
auto data = std::make_shared<ThreadData>(.....);
auto thread = std::thread([data]() {
.....
.....
.....
});
data->please_stop = true;
thread.detach();
data.reset();
}Просто циклическая зависимость между объектом и ассоциированным с ним потоком (и вынужденная циклическая ссылка любого типа — умная, слабая, или вообще голая) добавляет шарма.
Но вот в моём коде нет циклической зависимости. Объектом data владеют main() и поток. (Хотите сделать RAII — просто переложите локальные переменные в отдельную структурку).
Потоком владеют main() и система.
Главным потоком main владеет система.
Граф упорядочен, а зомби есть.
Между please_stop и прохождением любой контрольной точки может пройти сколь угодно много времени в ожидании или в интенсивной работе, которая — по нашей хотелке — не должна была бы совершаться.
Значит, надо нашу хотелку чем-то подтверждать. Переделывая всю архитектуру функции рабочего потока.
Колхозить семафоры на подсчёте ссылок — это ОЧЕНЬ ПЛОХАЯ ПРАКТИКА. Ваша программа будет работать ровно до того момента, как вы где-нибудь локально не подарите кому-то ссылку на сторону.
thread_ = std::thread([shis = enable_shared_from_this()]() {
{ auto shat = shis;
if (shis.use_count() == 1) return; // никогда
}
{ Partner partner(shis);
if (shis.use_count() == 1) return; // зависит от логики партнёра
}
});Ну и раз мы вообще делали detach (а обнуление умного указателя — это ни что иное, как отцепка нас от объекта, поэтому — какое-такое раи-шмаи?), то
- возможно, мы намеренно хотели создать поток, живущий своей жизнью
- возможно, нам нужна рукоятка для явной просьбы остановить поток (и дальше пусть он живёт своей жизнью, сколько сочтёт нужным)
{
auto zombie = SteppedZombie::create();
...
zombie->please_stop(); // взводит флажок
}- а если уж на то пошло, то можем такую взводилку сделать раи
class zombie_lock {
shared_ptr<Zombie> zombie;
bool ultimative;
public:
zombie_lock(shared_ptr<Zombie> z, bool u) : zombie(z), ultimative(u) {}
~zombie_lock() {
if (zombie) {
zombie->please_stop();
if (ultimative) { zombie->join(); }
}
}
};
...
int main() {
{ Zombie::create(); } // этот будет жить сколько влезет
{ zombie_lock zl(Zombie::create(), false); } // этот попросит сдохнуть
{ zombie_lock zl(Zombie::create(), true); } // этот даже дождётся
}Где тут enable_shared_from_this? Нигде, скажете вы? А я скажу — неважно где, может, везде, просто это не имеет значения.
А так-то да, просто сделать detached-поток никто не запретит. И иногда именно так и задумано.
Всё понятно, кривые кирпичи.
Примеры буста показывают, что операции там завершатся до выхода из main.
И посыл именно такой: как детачнуть объект и перестать о нём заботиться, при том, что он будет жив ровно столько, сколько надо.
А жив он будет, пока крутится цикл ioc.run(). И лишние ссылки (то есть, все ссылки) помрут по завершении этого цикла, перед выходом из main().
Разница в том, что у asio выполнение задач явно вынесено, а стандартные потоки живут глубоко под капотом, и доступ к ним — через разрозненные хэндлы.
"Если вы сделали поток, который владеет объектом, который владеет хэндлом этого потока, то — скорее всего, вы прострелили себе ногу".
"Если вы сделали поток и детачнули его, то — скорее всего, вы прострелили себе ногу".
Потому что кроме лично вас, потоком владеет только система, причём владеет довольно просто: выходит из main() и громко падает, если там были недобитые потоки.
А не "если вы стали использовать умные указатели".
Почему не "если вы стали использовать потоки"?
Посыл-то у статьи был правильный:
1) enable_shared_from_this облегчает создание кольцевых ссылок, — будьте аккуратнее
2) помните, что детач объектов и/или потоков не убивает объекты, — продумывайте способы управления временем их жизни
3) помните, что потоки останавливаются не мгновенно, — продумывайте задержки и последствия асинхронности
Но вместо "продумывайте" начинаются обвинения и велосипеды.
потоком владеет только система, причём владеет довольно просто: выходит из main() и громко падает
Скорее «тихо», чем «громко».
обвинения и велосипеды
Где обвинения?
Где велосипеды?
будьте аккуратнее
Как именно следует быть аккуратнее?
Статью я начал с разбора именно первого приходящего в голову способа (который присутствовал в комментах к первой части статьи): если собрался закольцеваться — не бери сильную ссылку, а бери слабую (эта фраза похожа на «собрался вылить чашку кофе на клавиатуру — накрой её чем-нибудь», ну да ладно, с этой частью разбираться я не буду). Отлично, берём слабую ссылку — и видим, что сложности есть. И как по мне, сложности неочевидные.
Опять же, посмотрите на название статьи. Я обещал в ней показать, как можно починить код из моей первой статьи. И вроде показал — несколькими способами, с подробным разбором недостатков каждого из них. Я что, обещал что ли «показать всемогущий предохранитель от простреливания любых ног»? Или может обещал «общую теорию стреляния в ноги»? Или может «общую теория написания хорошего кода»?
Я заставляю кого-либо чинить свой код именно так? Вроде нет.
Я заявляю, что эти методы будут пригодны для каждого конкретного случая? Вроде нет.
Я заставляю кого-либо не продумывать? Вроде тоже нет.
Кому эта статья может быть полезна? Тому, кто обнаружил в своём коде подобные паттерны, и не очень понимает, как это разруливать.
Вам достаточно указать на проблему, а что с ней делать Вы и сами разберётесь — ну что ж, вам эта статья не принесла пользы. Но только зачем Вы её читали, если из заголовка, соответствующего содержимому статьи, могли понять, что и сами разберётесь?
Что не так?
Где обвинения?
Да прямо в статье и в ваших комментариях, где вы утверждаете, что enable_shared_from_this вредно.
Где велосипеды?
В вашем коде. Вы сперва детачнули объект с потоком, а потом героически придумываете костылики, как загасить поток по косвенным признакам.
Если хотели продемонстрировать, что случайный программист так же бессистемно будет тыкаться, велосипедить и получать схожие проблемы, — то поздравляю, это получилось.
Что не так?
То, что вы не захотели посмотреть в корень проблемы, а продемонстрировали, как можно весело бегать по граблям, изобретая всё новые неудачные решения.
А всё началось с постулата о том, что shared_ptr — это RAII. Ну, потому что это так показалось и так захотелось.
Да прямо в статье и в ваших комментариях, где вы утверждаете, что enable_shared_from_this вредно
И Вы пытались мне оппонировать первым своим комментарием?
Так у Вас там методологическая ошибка: показывать ещё 5 способов нанести точно такой же вред, но без shared_from_this — это не доказательство безвредности shared_from_this.
Вы сперва детачнули объект с потоком
Не путайте форму с содержанием. На бусте Вы точно такие же спецэффекты сделаете и без detach. Просто примеры с stl намного проще для читателя, чем примеры с бустом.
Если хотели продемонстрировать, что случайный программист так же бессистемно будет тыкаться, велосипедить и получать схожие проблемы, — то поздравляю, это получилось.
У каждого из приведённых вариантов — своя техническая применимость и своя удалённость от начального варианта (читай «стоимость»).
А всё началось с постулата о том, что shared_ptr — это RAII. Ну, потому что это так показалось и так захотелось.
Можете продлить историю в прошлое, например так:
-3. Попросили либу.
-2. Либа дала нечто со static create(), возвращающей unique_ptr — самое что ни на есть RAII, и документировала, что для остановки надо просто уничтожить.
-1. Пользователь пожаловался, что иногда крэшится.
0. Либа расследовала. Стала использовать enable_shared_from_this и возвращать shared_ptr из static create(). Так как всё вокруг auto и у unique_ptr ограничений больше, чем у shared_ptr — пользователь даже ничего и не заметил, просто принял фикс. А если бы даже и заметил — он всё равно знает, сколько он сделал копий.
1. Тут начинается первая часть статьи.
И чтобы понять, что это не совсем выдумка, ответьте на простой вопрос: зачем (или откуда) в примерах буста shared_from_this?
изобретая всё новые неудачные решения
Жду pull-request с удачным решением. Обязательно непогрешимым и единственно верным, а ещё желательно очень дешёвым.
У меня доказательство бесполезности использования или отказа от enable_shared_from_this как волшебной пули. Если стрельба в ногу уже состоялась. Ну, выстрелите себе не в ступню, а в бедро. "И вот радио есть, а счастья нет".
Зачем/откуда в примерах буста shared_from_this — очевидно же, для того, чтобы можно было детачить задачи. Fire-n-forget.
Поскольку все задачи джойнятся по завершению io_context::run(), — ситуация не выходит из-под контроля, как с настоящими потоками. Поэтому это безобиднее, чем насоздавать потоков и детачить непосредственно их.
А насчёт удачного и непогрешимого решения, так по этим граблям давно уже пробежалась ява. Там, естественно, сборщик мусора работает иначе, чем плюсовый подсчёт ссылок, поэтому сценарии возникновения зомби отличаются.
Но главное, что их всех объединяет, это содержательная работа, возложенная на деструктор. Файл там закрыть, поток заджойнить, вот это всё…
И в той же яве пришлось переизобрести плюсовый детерминизм. Добавив ещё одну ручку явного закрывания, и конструкцию дёрганья этой ручки в scope.
Тот самый try-with-resources.
И аналогично — IDisposable + using в шарпе.
Здесь ровно такое же решение. enable_shared_from_this решает только задачу целостности памяти, а управление активностью, пожалуйста, делайте самостоятельно, согласно вашему ТЗ.
В силу того, что ваше ТЗ не специфицировано, то единственно верного решения нет. Специфицируете — дам его.
Намётки уже предложил: scoped-обёртки, которые дёргают ручки.
У меня доказательство бесполезности использования или отказа от enable_shared_from_this как волшебной пули
Какой-то очень странный разговор получается:
Я: — Топор опасен, им можно палец случайно отрубить. Произойдёт это при таких-то и таких-то условиях. Будьте осторожны при работе с топором.
Вы: — А ещё палец можно случайно отрезать болгаркой (вот пример). А ещё — деревообрабатывающим станком (вот ещё пример).
Я: — Да, можно. И что с того?
Вы: — Вы такие статьи пишете, потому что обобщать не умеете! Всей картины не видите! Зачем про топоры, надо про пальцы писать!
Я: Гмммм…
Вы: Ну я же только что доказал, что топор безопасен!
Я: Странно, я не заметил.
Вы: Не важно, будете Вы использовать топор или нет! Это всё равно не спасёт от болгарки! И от деревообрабатывающего станка тоже не спасёт! И ещё от гангрены!
Зачем/откуда в примерах буста shared_from_this — очевидно же, для того, чтобы можно было детачить задачи.
Я не это спрашиваю.
Вы мне начали рассказывать о том, как работают примеры буста и почему там всё корректно. Ну так вот почитайте внимательно и скажите: что будет, если shared_from_this просто заменить на this? Конкретно в этих примерах с конкретно этим main.
ява
шарпе
Зачем в комментах к статье, опубликованной в хабе «c++», рассказывать про костыли, которыми пришлось подпирать какие-то другие языки целиком, чтобы с подобным справляться?
scoped-обёртки, которые дёргают ручки
А кто мешает сразу выдать нечто, что само дёрнет за ручки?
Зачем приводить решения с явным дёрганьем ручек в статье, в которой в первую очередь рассматривается, как не дёргать явные ручки?
(у меня в гите есть ветка с явным дерганьем ручек, но я её только упомянул в статье как сложное решение, лишенное изящества)
Если бы Ваше решение было разумным — то при использовании stl сейчас мы бы писали что-то вроде:
auto data = std::scope_guard<std::make_shared<Data>(params)>;
auto data = std::scope_guard<std::make_unique<Data>(params)>;
Неужели это удобно?
В силу того, что ваше ТЗ не специфицировано
Всё специфицировано.
Нужно нечто:
— ожидающее данных от длительного процесса;
— отправляющее данные в listener по мере их прихода;
— обладающее свойствами RAII — в том числе в части защиты от исключений;
— допускающее уничтожение до завершения работы;
— уничтожение должно иметь смысл «освобождение ресурсов при первой возможности и строгое прекращение отправки данных в listener»;
— минимально воздействующее на главный поток (чтобы все методы были неблокирующими, включая создание и уничтожение).
Ненене.
- Топор опасен! Топором можно палец отрубить! Если вы покупаете топор, то, скорее всего, собираетесь отрубить себе палец!
- Если руки кривые, то можно и топором отрубить, и болгаркой.
- При чём здесь болгарка, если топор — антипаттерн!!!
При чём здесь ява и шарп — да при том, что они столкнулись с теми же проблемами и сделали оргвыводы. На уровне дизайна языка.
Там, где в плюсах достаточно сделать поддержку на уровне пользовательских библиотек, — и потому всё отдаётся на откуп прямо- или криворуким авторам библиотек.
Насчёт удобно-неудобно, я всё равно буду настаивать на примерах на шарпе
// неудобно
using (Some data = CreateActiveInstance()) { // лишняя писанина
...
...
...
} // лишняя скобка
// плюс ограничения на право передачи data за пределы блока
// удобно
Some data = CreateActiveInstance();
...
...
...
// если забыли, то и хрен с ним, мусороуборщик приде порядок наведеНаписали scope_guard — выразили своё намерение о поведении объекта.
Не написали — выразили намеренный пофигизм.
Из актуальных вопросов:
— что там с примерами буста?
— почему же Комитет по стандартизации c++ принял умные указатели сразу с поведением scope guard по умолчанию?
Никто не заставлял вас приплетать аналогии с топором. Не флудите и не флудимы будете.
Из актуальных вопросов:
Про буст: это простой пример, в котором enable_shared_from_this убирает всю ненужную и несущественную головную боль, связанную с ручным управлением временем жизни. Как написать fire-n-forget задачку, иллюстрирующую работу с http.
Если хочется сложного поведения, придётся делать сложные решения.
Тем более, что все хвосты будут подчищены при выходе из run().
Да, сокетные программы могут зависать в ожидании. И это известный вектор атаки, — например, установить соединение и долго и нудно пихать туда данные, занимая канал. Такое зомбирование, и никакие шаред поинтеры не спасут. И всё это — за рамками примера из документации буста.
Про комитет стандартизации: что?! Выход из контекста не уничтожает объект, если на него есть внешние указатели.
То, что вы о них можете не знать, — не означает, что автор библиотеки их не сделал, намеренно (например, зарегистрировал в глобальной табличке) или случайно.
В случае с потоками так и произошло: система владеет потоками, потоки владеют связанными аргументами своих функций.
Пожалуйста, отличайте хотелки от дизайна языка.
enable_shared_from_this убирает всю ненужную и несущественную головную боль
Я не понимаю, почему Вы всё уходите от прямого ответа на прямой вопрос: что будет, если в примерах буста заменить shared_form_this на this?
Выход из контекста не уничтожает объект, если на него есть внешние указатели
Стопстопстоп.
По умолчанию при выходе из scope происходит нечто существенное. Для unique_ptr — одно, для shared_ptr — другое. Непрошено, само собой. Чтобы при выходе из scope ничего не происходило — это надо специально просить.
Почему не наоборот? По Вашей логике, поведение по умолчанию должно быть как у сырого указателя: не просили — ну так ничего и не произошло. А чтобы что-то происходило — надо специально просить. Например, заворачивая в условный std::scope_guard или явно дёргая ручку.
Теперь про ТЗ.
1) Прочитайте свою статью и найдите, где оно там написано :) Раскидали намёки по всему тексту.
2) В вашем ТЗ есть неувязка:
- уничтожение должно иметь смысл «освобождение ресурсов при первой возможности и строгое прекращение отправки данных в listener»
- минимально воздействующее на главный поток (чтобы все методы были неблокирующими, включая создание и уничтожение****)
Как вы себе это представляете при многопоточности?
Если поток решительно собрался что-то отправлять, а вы его пытаетесь остановить, то у вас в любом случае с момента входа в процедуру остановки до момента получения потоком этого сигнала может пройти сколько угодно времени. Когда ещё эта первая возможность наступит.
Если имеется в виду, что по выходу из процедуры остановки-и-уничтожения отправка прекратилась, то — это именно блокирующий метод.
Предлагаю ввести иные утверждения:
- есть неблокирующая команда завершения-по-возможности; можем дать какие-нибудь реалтаймовые гарантии её исполнения (скажем, в течение 1мкс)
- есть блокирующая команда ожидания завершения; можем добавить версию с таймаутом
Дополнительно можем ввести требование ожидания завершения всех объектов, созданных в данном контексте, без прямого доступа к их хэндлам.
Если контекст глобальный, то такой мега-джойн производится где-нибудь в atexit().
Если мы умеем создавать отдельные контексты, то каждый из них в своём деструкторе сделает мега-джойн.
Да, блокирующий.
Да, с возможностью диагностики таймаутов.
Да, при выявленном таймауте можно эскалировать проблему вплоть до вызова terminate(), по вкусу.
Как мы видим, здесь ещё нет слов про управление памятью.
Но уже есть слова о том, что ручек, протянутых к объекту, более, чем две — и потому их нельзя раскидать по двум штатным ручкам — конструктору и деструктору, без потерь. Мы или неблокирующий вызов потеряем, или блокирующий…
Разве что делегировать блокирующие вызовы деструктору контекста — или ещё какому-то стоп-зе-ворлд уборщику.
Если такое устроит, то написать реализацию будет несложно.
Следовательно, потенциальная блокировка главного потока из-за конфликта с уже начавшейся отправкой мало того что даже в худшем случае очень мала, так ещё и очень маловероятна (но очень вреднА, если стрельнет не в ту сторону — поэтому глушить надо строго).
Под блокирующим методом я понимаю блокировку на время порядка того процесса, ради ожидания которого мы всё это затеваем (единицы — десятки секунд). И вот это недопустимо.
Ресурсы не очень важны, важно удобство пользования + корректность.
То, что у Вас помечено "****" — да, существенная часть задачи.
Более-менее понятно?
Сказ об опасном std::enable_shared_from_this, или антипаттерн «Зомби» — разбор полётов