Destructor-Handling bei Exception aus einem Constructor

[shft]

Hallo @all

class A
{
public:
	A()
	{
		throw std::exception( "test" );
	}

	~A()
	{
		cout << "wird trotzdeem aufgerufen!";
	}
};

int main()
{
	try
	{
		A a;
	}
	catch( std::exception& e)
	{
		cout << e.what();
	}
}

Wenn ich diesen Code ausführe, wird der Destruktor von A nicht aufgerufen.
Soweit ich das verstanden habe, wird ein Destruktor jedoch aufgerufen, sobald eine Exception außerhalb des Konstruktors geworfen wurde.
Da aber der Destruktor oft wichtige! Aufräumarbeiten erledigt, wäre es gut, wenn dieser auch aufgerufen werden würde, sollte eine Excpetion, im Konstruktor geworfen werden. Geht das? Oder muss ich folgendes schreiben:

[class A
{
public:
	A()
        try
	{
		throw std::exception( "test" );
	}
        catch( ... )
        {
            // Aufräumen
            throw;
        }

	~A()
	{
		cout << "wird trotzdeem aufgerufen!";
	}
};

mfG

shft

[Nathan]

shft schrieb:

Wenn ich diesen Code ausführe, wird der Destruktor von A nicht aufgerufen.

Richtig. Das könnte schlimme Folgen haben, da das Objekt u.U. nicht vollständig konstruiert wurde. Dann sind im Desktor nicht all invariants erfüllt und das Cleanup funktioniert evtl. nicht.

Oder muss ich folgendes schreiben:

Ja. Besser wäre es allerdings, zu vermeiden, in so eine Situation zu kommen.
Es wird zwar nicht der Destruktor von A aufgerufen, wohl aber die Destruktoren aller vollständig konstruierten Member. D.h. hat A einen std::string und einen std::vector, die in dieser Reihenfolge konstruiert werden, wird der Destruktor von std::string aufgerufen, wenn der Konstruktor von std::vector scheitert.
Versuche also die Rule of Zero zu verfolgen, und sorge durch Nutzen von Standardbiliotheksklassen die Notwendigkeit eines eigenen Destruktors zu eliminieren.

[shft]

Ah ok danke
Ich war zuerst ein bisschen erschrocken, dass evtl auch die Destruktoren der Member nicht aufgerufen werden würden, Stroustrup sei dank, habe ich mich getäuscht

[großbuchstaben]

wenn man im Konstruktor eine exception in Abhängigkeit von irgendwelchen Zustände werfen will, es also eine Bedingung, an der man erkennen kann, daß was schiefläuft, dann kann man doch aufräumen, bevor man wirft:

if (!DoSomethingWhereSomethingCanGoWrong(bla))
{ 
  CleanupWhatHasBeenCreatedUpToNow();
  throw exception();
}

[hustbaer]

@großbuchstaben
Man kann, aber ich würde sagen dass das der komplett verkehrte Ansatz ist. Das Problem entsteht bloss durch das nicht-Aufteilen einer zu grossen Klasse in kleinere. Workarounds wie "aufräumen vor dem Werfen" verschlimmern die Sache bloss noch durch unnötige Komplexität. Mal ganz davon abgesehen dass sie total brüchig sind, weil jede kleine Änderung neue Stellen schaffen kann wo Exceptions fliegen könnten (irgendwo in einer Unterfunktion einer Unterfunktion die im Ctor aufgerufen wird) - was dann zu einem Fehler/Leak führt.

Und Workarounds dieser Art lenken den Blick vom eigentlichen Problem ab, nämlich davon dass man eine Klasse hat die für zu viel zuständig ist.

@shft
Es werden alle Objekte zerstört die vollständig konstruiert wurden.
D.h. Member werden zerstört, da die Member für sich ja vollständig konstruiert wurden. Ebenso werden vollständig konstruierte Basisklassen zerstört.

Die einfachste Lösung für das Problem: alles so weit runterbrechen dass jede Klasse nur mehr genau eine "Resource" kontrolliert.
Bzw. allgemeiner: genau eine Sache macht ("do") die der Konstruktor rückgängig machen muss ("undo").

Anstatt also z.B. zwei dynamisch angeforderte Arrays in einer Klasse X zu verwalten, bastelt man sich erstmal eine "dynamisches Array" Klasse A, die genau ein solches Array konstruiert.
Und verwendet dann zwei A Member in der Klasse X.
Bzw. wenn es um dynamische Arrays geht macht man das natürlich nicht, sondern verwendet gleich std::vector .

Bei anderen Dingen allerdings, wo es noch keine fertigen RAII Wrapper gibt, schreibt man sich diese einfach selbst. Wobei man ruhig den Ball flach halten kann. Je nach Anwendungszweck kann es vollkommen ausreichend sein eine struct mit lediglich einem public Datenmember und einem Destruktor zu haben. "Ist zu aufwendig" ist also keine Entschuldigung dafür es falsch zu machen. (Erwähne ich nur weil ich dieses "Gegenargument" schon öfter gehört habe.)

[großbuchstaben]

seine Frage war, wie man aufräumt, wenn im Konstruktor eine exception geworfen wird.

Meine Antwort ist: Räum' auf, bevor du die exception wirfst.

Klassendesign war hier ja nicht gefragt.

[amlog]

großbuchstaben schrieb:

seine Frage war, wie man aufräumt, wenn im Konstruktor eine exception geworfen wird.

Meine Antwort ist: Räum' auf, bevor du die exception wirfst.

Klassendesign war hier ja nicht gefragt.

Wenn jemand fragt, "wie mache ich am besten Selbstmord", gibst du ihm dann eine Anleitung oder versuchst du, ihn davon abzuhalten?

[hustbaer]

großbuchstaben schrieb:

Meine Antwort ist: Räum' auf, bevor du die exception wirfst.

Das hab' ich schon verstanden. Ist bloss die falsche Antwort.
Die richtige ist: gar nicht, denn es hat dort nichts aufzuräumen zu geben.

[TDDler]

hustbaer schrieb:

großbuchstaben schrieb:

Meine Antwort ist: Räum' auf, bevor du die exception wirfst.

Das hab' ich schon verstanden. Ist bloss die falsche Antwort.
Die richtige ist: gar nicht, denn es hat dort nichts aufzuräumen zu geben.

hustbaer hat recht. Wenn die Klasse vernünftig entworfen ist, dann ist der Rumpf eines Konstruktors LEER.

A::A(void)
: x()
, y(bla)
, z(bla, bla)
{
return;
}

Wirft z() eine Exeption, dann werden x und y automatisch richtig gelöscht und dabei selbstverständlich der Destruktor von x und y aufgerufen. Deswegen müssen x und y Objekte sein und nicht nur einfache Zeiger. Denn der "Destruktor" eines Zeigers löscht nur sich selbst, aber nicht das Ding auf das er zeigt. Ein std::unique_ptr() dagegen löscht auch das Ding auf das er zeigt. Wenn x() der Kontruktor der Basisklasse ist, y ein Zeiger und bla ist "new Irgendwas", dann bleibt ein Irgendwas-Objekt übrig, auf das niemand mehr zugreifen kann (= memory leak), wenn z() eine Ausnahme wirft. Deshalb macht man sowas nicht, sondern nur mit einem smart pointer. Ggf. schreibt man sich eine eigene kleine Klasse, wie hustbaer bereits geschrieben hat. Von der wird dann auch der Destruktor aufgerufen, sollte das Objekt bereits vollständig initialisiert worden sein. Nur gibt es in der STL bereits so viele gute Lösungen, dass das praktisch kaum noch vorkommt.

Alles klar? Good luck.

[stl]

TDDler schrieb:

Nur gibt es in der STL bereits so viele gute Lösungen, dass das praktisch kaum noch vorkommt.

STL != Standardlibrary.

[tntnet]

großbuchstaben schrieb:

seine Frage war, wie man aufräumt, wenn im Konstruktor eine exception geworfen wird.

Meine Antwort ist: Räum' auf, bevor du die exception wirfst.

Klassendesign war hier ja nicht gefragt.

Es ist durchaus sinnvoll, einen Schritt zurück zu gehen und schauen, warum die Frage überhaupt auftaucht. Häufig gibt es bessere Lösungen, die das Problem von vornherein vermeiden. Und das ist hier so ein Fall, wo es sich lohnt zu überlegen, wie man das Problem elegant vermeiden kann.

[großbuchstaben]

TDDler schrieb:

Wenn die Klasse vernünftig entworfen ist, dann ist der Rumpf eines Konstruktors LEER.

im Idealfall ja. In der Praxis wäre ich mit solchen Pauschalbehauptungen vorsichtiger, da gibt es öfters mal Randbedingungen und Vorgaben, die es nicht erlauben zu programmieren wie in example 4.3.2 deines C++ Lehrbuchs.

ändert aber auch nix am Sachverhalt: Klassendesign hin oder her, gefragt war nach dem Aufräumen bei exception im Konstruktor und meine Antwort ist "erst aufräumen, dann werfen" und fertig

Deswegen müssen x und y Objekte sein und nicht nur einfache Zeiger.

"müssen" muß gar nichts. Wenn ein langjähriges 100k+ LoC Projekt zeigerbasiert designt ist, und jemand finge in Hierarchieebene 6 plötzlich mit Referenzen oder smart pointers an ...

[Finnegan]

großbuchstaben schrieb:

im Idealfall ja. In der Praxis wäre ich mit solchen Pauschalbehauptungen vorsichtiger...

Gut, das du das ansprichst. Solche scheinbar in Stein gemeißelten Regeln wie "man darf nie...", "man muss immer..." liest mal leider zu häufig, und so wie sie formuliert sind, sind sie fast immer falsch. Korrekter sollte es eigentlich heißen "Man sollte bevorzugt..., weil..., es sei denn man hat einen guten Grund es anders zu machen". Das gilt für fast alle dieser No-Gos, manchmal (jedoch meistens nicht!) sind sie die beste und eleganteste Lösung für ein Problem.

Kürzlich habe ich sogar mal folgendes in Produktivcode geschrieben:

...
std::shared_ptr<Type>* ptr = new std::shared_ptr<Type>(other); 
...
delete ptr;
...

Und obwohl das vermutlich auf den ersten Blick viele entsetzt aufschreien lässt, tat ich das aus gutem Grund, und habe den Code ohne schlechtes Gewissen eingecheckt

Natürlich sollte man besonders Anfänger immer darauf hinweisen, dass man gewisse Dinge besser sein lässt, aber nicht ohne den Hinweis, dass es auch (seltene) Ausnahmen gibt (nicht dass sie sich nacher nicht trauen eine dieser goldenen Regeln zu verletzten, obwohl der Alternativcode undgleich unverständlicher, komplexer oder sogar unmöglich ist).

Finnegan

[proof by contradiction]

Finnegan schrieb:

Kürzlich habe ich sogar mal folgendes in Produktivcode geschrieben:

...
std::shared_ptr<Type>* ptr = new std::shared_ptr<Type>(other); 
...
delete ptr;
...

Und obwohl das vermutlich auf den ersten Blick viele entsetzt aufschreien lässt, tat ich das aus gutem Grund, und habe den Code ohne schlechtes Gewissen eingecheckt

Mehr Kontext bitte. Das kann so nicht stimmen.

[hustbaer]

großbuchstaben schrieb:

ändert aber auch nix am Sachverhalt: Klassendesign hin oder her, gefragt war nach dem Aufräumen bei exception im Konstruktor und meine Antwort ist "erst aufräumen, dann werfen" und fertig

Ja, und deine Antwort ist immer noch falsch.
Weil "erst aufräumen, dann werfen" in einem Programm mit Exceptions nix verloren hat. Weil es zu Problemen (=Bugs) führt. Weil es falsch ist.

[hustbaer]

..

[Finnegan]

proof by contradiction schrieb:

Finnegan schrieb:

...
std::shared_ptr<Type>* ptr = new std::shared_ptr<Type>(other); 
...
delete ptr;
...

Mehr Kontext bitte. Das kann so nicht stimmen.

ptr wird an eine C-API übergeben, die ich nicht modifizieren kann, und als Argument einen void* erwartet, der dann wiederum in einem asynchronen Aufruf (anderer Thread) als User-Parameter an eine von mir implementierte Callback-Funktion übergeben wird. Diese Funktion benötigt die Type-Instanz, und es ist erforderlich, dass diese Funktion im Kontext des Threads dieser C-API aufgerufen wird (es handelt sich um eine Operation auf einem von der C-Bibliothek erzeugten OpenGL-Kontext, auf den ich keinen direkten Zugriff habe, der aber innerhalb dieser Callback-Funktionen aktiviert ist).

Nun könnte es passieren, das die Type-Instanz gelöscht wird, bevor die C-Callback-Funktion (asynchron) aufgerufen wird (wann das passiert, ist von - ebenfalls asynchronen - Ereignissen in anderen Threads abhängig, an die der shared_ptr weitergereicht wird).

Der Pointer auf den Shared Pointer dient dazu, die Type-Instanz so lange am Leben zu erhalten, bis die C-Callback-Funktion aufgerufen wurde (diese mach abschließend auch das delete auf dem Pointer). Wegen des o.g. void*-Arguments kann ich dieser API eben keinen shared_ptr übergeben, und weil die Type-Instanz sterben kann, bevor die C-Callback aufgerufen wurde kann ich auch keinen Type* übergeben ohne Gefahr zu laufen, dass ich Operationen auf einem Dangling Pointer mache.

In Code, über den ich vollständige Kontrolle habe, wirst du bei mir solche Konstrukte sicher nicht finden, aber manchmal machen auf den ersten Blick wahnwitzige "Patterns" doch Sinn.

Finnegan

[Finnegan]

hustbaer schrieb:

Ja, und deine Antwort ist immer noch falsch.
Weil "erst aufräumen, dann werfen" in einem Programm mit Exceptions nix verloren hat. Weil es zu Problemen (=Bugs) führt. Weil es falsch ist.

Vielleicht liegt ein Missverständnis vor, was hier verschiedene Leute unter "falsch" verstehen. Für mich ist ein Programm "falsch" wenn es nicht das korrekte Ergebnis liefert (UB oder nicht korrekter Algorithmus) oder wenn es syntaktisch falsch ist.

Recht gebe ich dir dahingehend, dass es so "hochgradig nicht empfehlenswert" ist, oder "mit höherer Wahrscheinlichkeit zu Fehlern führt". Natürlich sollte man möglichst vermeiden in Konstruktoren Exceptions zu behandeln, aber manchmal hat man keine andere Wahl, und dann (und nur dann!) sollte man eben aufräumen und dann werfen.

Finnegan

[proof by contradiction]

Würde ich trotzdem so schreiben:

auto ptr = std::make_unique<std::shared_ptr<Type>>(other); // oder sogar std::move(other)
... // hier könnte immer Code eingefügt werden, der wirft
c_api_with_callback(ptr.release());

Dein Code mag zwar in der Situation korrekt sein, beruht aber auf Subtilitäten (noexcept zwischen new und Callback-Aufruf) und geht deshalb sehr leicht kaputt. Deshalb lieber garantiert ausnahmensicher programmieren.

[Finnegan]

proof by contradiction schrieb:

Würde ich trotzdem so schreiben:

auto ptr = std::make_unique<std::shared_ptr<Type>>(other); // oder sogar std::move(other)
... // hier könnte immer Code eingefügt werden, der wirft
c_api_with_callback(ptr.release());

Dein Code mag zwar in der Situation korrekt sein, beruht aber auf Subtilitäten (noexcept zwischen new und Callback-Aufruf) und geht deshalb sehr leicht kaputt. Deshalb lieber garantiert ausnahmensicher programmieren.

Wenn dort Code steht, der werfen kann ja, allerdings wird der Shared-PointerPointer ausschliesslich für diesen einen Aufruf der C-API erzeugt. Die beiden Zeilen oben würden im Code direkt untereinander stehen, und daher in diesem Fall eklatant das "keep it simple, stupid!"- und das "programmier' nicht für Code, der hier irgendwann vielleicht mal stehen könnte"-Prinzip verletzen.

Generell ist dein Hinweis jedoch sinnvoll, aber nur wenn dazwischen auch tatsächlich Code steht (Funktionsaufrufe) bei dem man sich nie ganz sicher sein kann

Finnegan