RAII и необрабатываемые исключения

[Door]

If no matching handler is found, the function std::terminate() is called; whether or not the stack is
unwound before this call to std::terminate() is implementation-defined (15.5.1).

Вот это поворот. Спасибо. Неожиданно. У меня шок

[dmitrmax]

какой какашный код у конструктора класса LockNet. Что будет если new Network бросит исключение?

[FoxCanFly]

Обычно, если new бросает исключение, упасть является вполне логичным исходом. Разве что какой нибудь лог записать (хотя и этого может уже не получиться, если места в куче не осталось совсем)

[encyclopedist]

Это если мы получим std::bad_alloc. Но Network может например подключаться к этому url. И тогда он мог бы кинуть timeout или no route to host, которые вполне можно было бы обработать.

[beeruser]

>> И тогда он мог бы кинуть timeout
Конструктор, который висит пока таймаут сетевого соединения не пройдёт?
Кхм… смело.
В отдельном потоке конечно не так страшно.
Но лично я был бы удивлён таким поведением.

[encyclopedist]

Да, это был бы не лучший дизайн.

Но, например тут на хабре был автор, который упорно предлагал всё приложение (насколько я помню, там веб-сервер был) целиком запихивать в конструктор.

[dmitrmax]

а вы не думали, что не new может бросить, а конструктор Network? Мы тут вообще про raii, а не про то, что делать с каждым конкретным исключением

[icelaba]

Собственно конкретно в этом примере даже если new бросит исключение ничего страшного нет,
никакой беды не возникнет от того что деструктор ~LockNet не сработает.
Объекта Network и не существовало и нечего деструктить.

Но то что такой стиль написания кода ведет к страшным ошибкам и гемороям это несомненно.
Например если в конструкторе LockNet перед new Network будет получение какого либо хендла который надо закрыть в деструкторе —
то оппа можно получить системный лок ресурса.

Ну а геморои это например желание избавиться от new — получим двойную инициализацию и тп.

[sborisov]

Любой ресурс, выделяемый по new, должен оборачиваться отдельным локальным объектом.
Даже если будет исключение, после того как вы уже захватили другой ресурс, захваченный ресурс будет удалён благодаря RAII.

Посмотрите примеры у Страуструпа.

[encyclopedist]

В вашем примере вообще не совсем понятно зачем нужен new.

[sborisov]

Для объяснения что такое RAII

[icelaba]

А может вместо смотреть примеры придумаете для конкретно ЭТОГО примера зачем его оборачивать?

Например я уже много лет не создал ни одного указателя не обернув его СмартПоинтером того или иного вида,
и для меня вообще любой код вида a = new A() по умолчанию «плохой»,
но это не значит что «плохой» код приводит к ошибкам всегда — просто он не лежит в «моей» идеологии того как должен выглядеть код.

Аналогично Страуструп приводит примеры как правильно писать код в «общем» не рассматривая частные случаи.

[sborisov]

Оборачивая его локальным объектом, мы симулируем работу умного указателя, хоть и упрощённого донельзя :)

[icelaba]

Речь не об этом.

Речь о том что инициализировать объекты,
без видимых причин,
внутри тела конструктора а не в initializer list
очень! очень! плохой стиль написания кода.

И если вы до кучи внутри тела конструктора обернете объект локальным объектом-мембером,
то вы получите двойную его инициализацию со всеми вытекающими.
class A{
A() {
//ВОТ ТУТ «a» УЖЕ инициализирован дефолтным конструктором
}

Obj a;
}

[sborisov]

Какое дефолтное значение у указателя? Он будет инициалирован случайным значение скорее всего.
На практике, нужно всегда использовать список инициализации. Данный код написан для наглядности.

[sborisov]

Повторюсь ещё раз, вы все смотрите на вводную часть статьи, которая носит демонстрационный характер, показывающая «на пальцах» что такое RAII.
Основная часть ниже, там и умные указатели и никаких new!
Тот код прошёл вашу цензуру? :)

[icelaba]

Я как const nazi докопался бы и до второй части статьи :-) там в этом плане есть к чему :-)

[sborisov]

Идеал, как всегда недостижим.
Но я надеюсь, моя статья об этом скользском месте RAII окажется полезной некоторым разработчикам.

[danfe]

Например если в конструкторе LockNet перед new Network будет получение какого либо хендла который надо закрыть в деструкторе — то оппа можно получить системный лок ресурса.

Верно, получение хендла в конструкторе — это известная проблема, и элегантного решения, по большому счету, нет. Особенно если хендл получаешь от сторонней библиотеки, которая не бросает исключений, или это вообще системный вызов; тривиальный пример: open(2).

[sborisov]

1. URL, который передаётся Network, создаётся из временного объекта, на стеке. Если в URL будет исключение, как локальный объект URL будет удалён.
2. Если в конструкторе объекта Network происходит исключение, объект считается несозданным, удалять нечего.

Цель примера была привести пример, как работает RAII, а не пример строгой гарантии исключений.

[sborisov]

Это будет означать, что объект не создан, почитайте внимательней, как работает RAII.

[dmitrmax]

Ок, я попытаюсь по другому написать, что я имел ввиду. Возьмем ваш класс и немного его усложним:

class LockNet{
public:
    LockNet(const Url &url){
        m_net1 = new Network(url);
        m_net2 = new Network(url);
    }
    ~LockNet (){
        delete m_net1;
        delete m_net2;
    }

private:

    Network *m_net1;
    Network *m_net2;
};

Положим теперь, что исключение брошено при инициализации m_net2. Проблемы понятны? Здесь вы уже даже не отделаетесь try-catch блоком в конструкторе.

Я просто к тому, что если берешь на себя ответственность писать про RAII, то надо следовать этому во всем. А то читатель возьмет себе в голову данный пример и начнет так писать управление ядерным реактором )

[sborisov]

См. комментарий выше.
habrahabr.ru/post/253749/?reply_to=8341835#comment_8341397

Любой ресурс, выделяемый по new, должен оборачиваться отдельным локальным объектом.

[Mingun]

По вашему примеру выходит, что вообще невозможно написать ни один умный указатель, так как где-то же все равно нужно будет написать new. И почему бы не поместить там рядом еще один new? А дальше что? Ну сделаем мы вместо двух new два умных указателя (собственной закваски, для наглядности). Залезем в них и повторим.

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

[dmitrmax]

по моему примеру про умные указатели вообще никаких выводов не следует. умный указатель проблему решает. И пример вообще не про new, и не про то, что new нельзя писать. Подумайте над ним внимательнее.

[Mingun]

А давайте теперь усложним ваш умный указатель и добавим в нем еще одно поле, чтобы он хранил два объекта. А теперь подумайте внимательней, что будет, если мы попрем общепринятые каноны и назовем этот ваш модифицированный указатель, ну, хотя бы, LockNet… Имена классов вообще ведь для человека придуманы, а чтобы пример не казался вам надуманным, давайте представим, что код пишет параноик и любитель острых ощущений, и специально именует все свои классы и переменные как можно более нелогичными именами.

[dmitrmax]

А… это вы, Штрилиц…

Во-первых, это не мой умный указатель. Во-вторых, усложнять там нечего: в моём примере и так хранятся два объекта. Метод математической индукции тут применим, а поэтому дальше усложнять не имеет смысла, т.е. этот пример не умаляет общности.

Если вы хотите поизвращаться, то я могу вам написать умный указатель LockNet, который успешно следит за двумя и более ресурсами. Но для этого, нужно хотя бы обнулить указатели в списке инициализации. В противном случае вы не сможете отделить мусор в значении указателей от реального указателя, когда будете делать delete в catch-блоке. Собственно, по большому счету, мой «наезд» заключался именно в этом.

[Mingun]

Не понял, при чем тут Штирлиц.

Во-вторых, усложнять там нечего: в моём примере и так хранятся два объекта.

В вашем примере вы предлагаете использовать умный указатель, который структурно выглядит так же, как исходный пример из статьи, напомню его здесь:

class LockNet{
public:
    LockNet(const Url &url){
        m_net = new Network(url);
    }
    ~LockNet (){
        delete m_net;
    }

    operator Network * (){
        return network;
    }

private:

    Network *m_net;
};

И на ваше предложение добавить в него второй член я точно также говорю вам добавить второй управляемый объект в класс умного указателя. Или вы хотите сказать, что в этом случае проблем не будет? А если не хотите, то зачем вы приплели сюда усложненный пример (опять же, напомню):

class LockNet{
public:
    LockNet(const Url &url){
        m_net1 = new Network(url);
        m_net2 = new Network(url);
    }
    ~LockNet (){
        delete m_net1;
        delete m_net2;
    }

private:

    Network *m_net1;
    Network *m_net2;
};

Если вас беспокоит, что в исходном примере не используются списки инициализации, так и что? Ну вылетит исключение в конструкторе LockNet, деструктор LockNet вызван не будет, и вызов delete над мусорным указателем m_net не произойдет, если вас это беспокоит.

В противном случае вы не сможете отделить мусор в значении указателей от реального указателя, когда будете делать delete в catch-блоке. Собственно, по большому счету, мой «наезд» заключался именно в этом.

Я что-то потерял вашу мысль… delete чего в каком catch?

[dmitrmax]

Как вы будете обрабатывать ситуацию, если полетит исключение при инициализации m_net2? m_net1 и в общем случае ещё N ресурсов выделено, их нужно удалить.

[Mingun]

В примере кода, из статьи, который мы тут обсуждаем, нет никаких m_net1 и m_net2 — они появились в вашем примере, с неправильностью которого никто и не спорит. А то интересный у вас подход — сначала изменим код автора, а потом скажем, что он неправильный.

Еще раз мое возражение к вашему:

какой какашный код у конструктора класса LockNet. Что будет если new Network бросит исключение?

в том виде, как он в статье приведен, код нормальный и со своей целью — показать пример использования RAII — он отлично справляется.

Если new бросит исключение, до инициализации Network дело даже не дойдет, а LockNet останется не инициализированный и его деструктор вызываться не будет.

Если конструктор Network бросит исключение, то оба объекта, и Network, и LockNet, останутся неинициализированными и их деструкторы вызываться не будут. Память, выделенная new для Network, в этом случае будет утекать как в реализации LockNet, так и умного указателя. В этом смысле вы правы — выделение память нужно делать в своей RAII обертке. Но для иллюстрации RAII это совершенно необязательно.

[MichaelBorisov]

Единственная проблема, которую я вижу в модели RAII — это исключения в деструкторах.

Иногда освобождение ресурса может пройти неуспешно. Но бросать исключения из деструктора в общем случае нельзя. Как минимум — если деструктор был вызван в процессе раскрутки стека при обработке другого исключения. В результате большинство деструкторов пишутся так, чтобы игнорировать ошибки. В некоторых ситуациях это может оставить среду в неправильном состоянии.

Другая проблема — освобождение блокировок. Допустим, есть класс MutexLock, конструктор которого захватывает mutex, а деструктор — освобождает. Все вроде бы хорошо, но если мутекс был освобожден в результате исключения, возникшего при работе с защищаемой структурой — ее состояние при освобождении мутекса может оказаться испорченным. Нужно либо принимать меры к откату всех изменений, либо иметь какой-то флаг «consistency», который будет проверяться при последующем доступе к этой структуре.

[sborisov]

В статье описывается проблема, когда исключения не перехватывается — RAII не работает.

[arabesc]

А в чем суть, что до вызова terminate может не случится разворота стека, но хотелось бы?

[sborisov]

По крайней мере, мне кажется такое впечатление складывается (стек будет всегда раскручен), после прочтения любой статьи про RAII, если объект будет завёрнут в умный указатель.

[arabesc]

Впечатление складывается из ложного интуитивного чувства, что в поисках подходящего обработчика исключения будет раскручиваться стек, хотя это нигде не обещается. Стандарт лишь говорит, что если обработчика нет, будет вызван terminate. Т.е. непосредственно RAII не при чем.

[alexeykuzmin0]

Я так понял, что как раз обычно все предполагают, что RAII у нас работает, а это может быть не так (то бишь, вызов деструктора не гарантирован). Это может вызвать проблемы, связанные с утечкой ресурсов, которые не освобождаются ОС автоматически при terminate (вроде, например, сокеты к таким относятся).

[shebeko]

Это, ваш пример не демонстрирует проблему с деструкторами.
Логично, что если main() не ловит исключение, оно перехватывается в std::terminate()
Важно понимать, что из std::terminate поток уже не возвращается. Поэтому логично, что до деструкторов дело не доходит.

Вместо того, чтобы давить исключение в testShared(), можно обернуть весь main()
Как-то так:

int main()
{
	try
	{
		vector<shared_ptr<Slot>> vec {make_shared<Slot>("0"), make_shared<Slot>("1"), make_shared<Slot>("2"), make_shared<Slot>("3"), make_shared<Slot>("4")};

		for (auto& x:vec)
				testShared(x);
	}
	catch(const std::exception& e)
	{
		std::cerr<<e.what()<<std::endl;
	}
	catch(...)
	{
		std::cerr<<"Unknown exception"<<std::endl;
	}

    return 0;
}

В В любом случае в этом есть смысл, т.к. вероятно вы хотите как-то хотя бы сообщать об ошибках в своей программе, прежде чем она тихо упадёт.
И тогда все деструкторы будут прекрасно отрабатываться.

[encyclopedist]

Кстати, в C++ есть такая конструкция, как Function try block (предназначен он изначально чтобы перехватывать исключения в списках инициализации конструкторов, их обычным способом не поймать).

int main() try
{
    vector<shared_ptr<Slot>> vec {
        make_shared<Slot>("0"), 
        make_shared<Slot>("1"), 
        make_shared<Slot>("2"), 
        make_shared<Slot>("3"), 
        make_shared<Slot>("4")};

    for (auto& x:vec)
        testShared(x);
    return 0;
}
catch(const std::exception& e)
{
    std::cerr << e.what() << std::endl;
    return 1;
}
catch(...)
{
    std::cerr << "Unknown exception" << std::endl;
    return 1;
}

[arabesc]

Замечу, что не совсем коректно говорить о перехвате исключений в catch-блоке конструктора, скорее там возможна их обработка, т.к. исключение невозможно полностью задавить и продолжить работу.

[sborisov]

Да, я про это написал в статье.

И рекомендует если вам нужно гарантированное удаление локальных объектов оборачивать код в функции main блоком try — catch (...), который перехватывает любые исключения.

[sborisov]

Переписал пример.
Так вывод работы RAII более наглядный. Спасибо.

[shebeko]

А с деструкторами тоже не всё так страшно.
Проблемы возникают только тогда, когда деструкторы выполняют код, который сам по себе может сгенерить исключение.

Например, пусть у нас есть транзакция, мы её оборачиваем RAII. Если коммит не проиходит, то происходит автоматический rollback() в деструкторе.

#include <iostream>
#include <vector>
#include <memory>
#include <string>
#include <exception>

class Transaction
{
	bool active;
public:
	Transaction() : active(false)
	{
		std::cout<<"begin transaction"<<std::endl;
		active=true;
	}

	~Transaction()
	{
		rollback();
	}

	void commit()
	{
		if(!active)
			return;
		std::cout<<"commit transaction"<<std::endl;
		active=false;
	}

	void rollback()
	{
		if(!active)
			return;

		std::cout<<"rollback transaction"<<std::endl;
		active=false;
	}
};

void do_something()
{
}

int main()
{
	try
	{
		Transaction tr;
		do_something();
		tr.commit();
	}
	catch(const std::exception& e)
	{
		std::cerr<<e.what()<<std::endl;
	}
	catch(...)
	{
		std::cerr<<"Unknown exception"<<std::endl;
	}

    return 0;
}

Вывод ожидаемый:

begin transaction
commit transaction

Теперь допустим в процессе обработки данных что-то пошло не так:

void do_something()
{
	throw std::runtime_error("processing data failed");
}

Транзакция нормально откатывается:

begin transaction
rollback transaction
processing data failed

Теперь допустим, что в процессе обработки данных исключение не генерируется, но commit() не происходит, скажем по логическим причинам в самой проге. Тогда автоматом вызовется rollback() из деструктора. И вот допустим уже в rollback() происходит исключение:

	void Transaction::rollback()
	{
		if(!active)
			return;
		throw std::runtime_error("something bad happens when we rollback transaction");

		std::cout<<"rollback transaction"<<std::endl;
		active=false;
	}

bool do_something()
{
	return false;
}

int main()
{
...
		Transaction tr;
		if(!do_something())
			return 1;
		tr.commit();
...
}

О боже! Мы не словили исключение в деструкторе и ничего не произошло. Мы успешно добрались до catch():

begin transaction
something bad happens when we rollback transaction

И вот только, если исключение в деструкторе генерируется в процессе обработки другого исключение, тогда у нас проблемы:

	void Transaction::rollback()
	{
		if(!active)
			return;
		throw std::runtime_error("something bad happens when we rollback transaction");

		std::cout<<"rollback transaction"<<std::endl;
		active=false;
	}

void do_something()
{
	throw std::runtime_error("processing data failed");
}

int main()
{
...
		Transaction tr;
		do_something();
		tr.commit();
...
}

Это весь вывод:

begin transaction

Что же делать? Если код в деструкторе достаточно простой, то обычно забивают и ничего не делают.
Можно просто глушить исключения в деструкторе, как многие рекомендуют.
Но я не считаю, что это хорошее решение. Т.к. в нашем случае, если транзакция нормально не откатится, то хорошо бы об этом знать.

Можно сделать так:

	~Transaction()
	{
		try
		{
			rollback();
		}
		catch(...)
		{
			if(!std::uncaught_exception())
				throw;
		}
	}

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

[Eivind]

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

Эта проблема решена в следующем стандарте — int std::uncaught_exceptions(). В конструкторе сохраняем текущее количество исключений, а в деструкторе сравниваем с сохраненным.

[stalkerg]

А что делать системам где нету исключений?

[mmatrosov]

Джон Калб говорил об этом на CppCon 2014 (ну и не только там): Exception-Safe Code, см. слайд 95. А так да, годное замечание, с ходу это неочевидно.