Hilfestellung für exception-Sicherheit und RAII



  • Hallo Leute,

    Nachdem ich nun mehrere Monate an einem Programm programmiert habe, ist mir an meiner Softwarearchitektur etwas aufgefallen, dass ich gerne optimieren möchte. Ich bin da ziemlich unsicher und hoffe, dass ihr mich eventuell ich die richtige Richtung lenken könnt. Aber erstmal von vorne. Mein Programm ist gewiserweise hierarchisch Aufgebaut. Ich habe einige Hauptmodule, die High-Level-Funktionen aufrufen. Diese rufen wiederum Low-Level-Funktionen auf, ...und diese arbeiten z.B. mit externen Bibliotheken wie OpenSSL.

    Hier ist mal die Darstellung meines Problems:

    void lowLevelFunction()
    {
    	try
    	{
    		throw std::string("irgendein fehler"); // irgendein Fehler ist aufgetreten
    	}
    	catch (std::string const &error)
    	{
    		//...Ressourcen freigeben und Loggen...
    		throw;
    	}
    
    	//...Ressourcen planmäßig freigeben...
    }	
    
    void highLevelFunction()
    {
    	try
    	{
    		lowLevelFunction()
    	}
    	catch (std::string const &error)
    	{
    		//...Ressourcen freigeben und Loggen...
    		throw;
    	}
    
    	//...Ressourcen planmäßig freigeben...
    }
    
    int main()
    {
    	try
    	{
    		highLevelFunction();
    	}
    	catch (std::string const &error)
    	{
    		//...Ressourcen freigeben und Loggen...
    	}
    	catch(..)
    	{
    	   //...irgendwas
    	}
    
    	return 0;
    }
    

    Wenn nun in den LowLevel-Funktionen eine exception geworfen wird, dann wird diese zunächst in der gleichen Funktion gefangen. Nun werden die Ressourcen freigegeben und der Fehler geloggt. Anschließend wird ein rethrow gemacht und diese Vorgang wiederholt sich in allen darüberliegenden Funktionsebenen.

    1. Problem: In jeder Funktion habe ich quasi 2 Aufräumvorgänge (beim normalen Beenden und bei exceptions)
    2. Problem: Ich fange momentan nur meine Fehlerklasse. Alle anderen exceptions werden erst in der main() gefangen --> Ressourcen werden also nicht immer freigegeben.
    3. Problem: Funktionen werden durch try/catch immer unnötig aufgeblasen (optisch).

    Die Lösung: RAII

    ... nur hört es sich irgendwie leichter an als ich es umsetzen kann. Ich finde in Foren teilweise sehr komplexe Lösungen, die ich allerdings nicht immer so ganz verstehe. Auch Boost nutze ich momentan nicht und möchte es nicht nur für diesen Zweck einbinden. Das Prinzip von RAII ist mir in etwa verständlich. Man erzeugt über eine Klasse einen ScopeGuard, der im Destruktor die zugewiesene Ressource freigibt.

    Nun mein Problem an der Sache (Beispiel):

    void Function()
    {
    	TYPE *meineVariable = eineOpenSslFunktion(...) // alloziert in irgendeinerweise Speicher für den Datentyp 'TYPE'
    
    	//... irgendwelcher code
    
    	TYPE_free(meineVariable); // ressource Freigeben
    }
    

    TYPE ist irgendein Datentyp. Im Falle OpenSSL gibt es diese zu Hauf. Jeder Datentyp hat da seine eigene Aufräumfunktion (teilweise unterschiedlich von den Parametern). Wie kann ich sowas am besten mit RAII umsetzen? Muss ich für jeden Datentyp extra eine ScopeGuard-Klasse definieren? Oder kann man einen allgemeinen ScopeGuard erstellen, bei dem man als Parameter angibt, wie die Ressource freigegeben wird. In etwa sowas:

    void Function()
    {
    	TYPE *meineVariable = eineOpenSslFunktion(...) // alloziert in irgendeinerweise Speicher für den Datentyp 'TYPE'
    	SG myScopeGuard(meineVariable, "TYPE_free(meineVariable)");
    
    	//... irgendwelcher code
    }	
    	~myScopeGuard --> führt nun irgendwie 'TYPE_free(meineVariable)' aus
    

    beste Grüße,
    SBond


  • Mod

    In diesem speziellen Fall, würde ich einfach einen unique_ptr mit eigener Freigabefunktion nehmen:

    // Irgendwo weit weg:
    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    
    // Konkret da, wo es gebraucht wird:
    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    
    // Rundum sorglose Benutzung von meineVariable, Aufräumen erfolgt automatisch
    

    Falls man es öfter benutzt, bieten sich ein paar clevere typedefs an.



  • vielen Dank. Werde es morgen früh mal testen. 😃



  • juhuu soweit funktioniert es 🙂

    Aber noch 2 kurze Fragen dazu:

    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    

    Warum eigentlich static? Ohne funktioniert es scheinbar auch.

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    

    Kann man das auch in zwei Zeilen zerlegen?

    irgendwie so:

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable();
    meineVariable(eineOpenSslFunktion(...), TYPE_closer);  // so geht es zumindest nicht
    

    Irgendwie komme ich "unique_ptr" und noch nicht so richtig klar. Oder kann man diese nur in eine Zeile definieren, die man dann im Editor höchstens umbrechen kann?

    Aktuell sind alle meine Funktionen nach folgendem Schema aufgebaut:
    Am Funktionsanfang werden alle verwendeten Variablen deklariert und (meist mit 0/nullptr) initialisiert.
    Die Wertzuweisung erfolgt dann, wenn die Variable zum ersten mal benötigt wird. Beispielsweise nach der Überprüfung der Funktionsparameter.

    void Funktion()
    {
    	TYPE *meineVariable = nullptr;
    
    	// ...irgendwelcher code...
    
    	meineVariable = eineOpenSslFunktion(...);
    
    	// ...irgendwelcher code...
    
    	TYPE_free(meineVariable);
    }
    

    viele Grüße,
    SBond



  • Funktionsanfang werden alle verwendeten Variablen deklariert und (meist mit 0/nullptr) initialisiert.
    Die Wertzuweisung

    Genau das sollte man in C++ nicht machen. Definiere eine Variable, wenn du ihr einen sinnvollen Wert geben kannst. Dann braucht man auch nicht auf nullptr testen.


  • Mod

    SBond schrieb:

    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    

    Warum eigentlich static? Ohne funktioniert es scheinbar auch.

    Weil ich mir dachte, dass der wahrscheinlichste Fall ist, dass dies global in einer Implementierungsdatei stehen wird, in der du das brauchst. Und da dann eben static, um das Ding nicht unnötig überall sichtbar zu haben.

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    

    Kann man das auch in zwei Zeilen zerlegen?

    Sollte machbar sein, wenn man ein paar Hürden überspringt, aber wieso das nicht sinnvoll ist, hat manni66 erklärt. Du willst ja gerade von dieser Art der Programmierung weg.



  • SBond schrieb:

    TYPE ist irgendein Datentyp. Im Falle OpenSSL gibt es diese zu Hauf. Jeder Datentyp hat da seine eigene Aufräumfunktion (teilweise unterschiedlich von den Parametern). Wie kann ich sowas am besten mit RAII umsetzen? Muss ich für jeden Datentyp extra eine ScopeGuard-Klasse definieren? Oder kann man einen allgemeinen ScopeGuard erstellen, bei dem man als Parameter angibt, wie die Ressource freigegeben wird.

    Machen kannst du beides. Wobei die Variante für jeden Datentyp eine eigene Klasse zu basteln langfristig meist angenehmer ist. Zumindest wenn man jeden Datentyp mehr als 1-2 mal benötigt.
    Bei generischen Scope-Guards (scoped_ptr, shared_ptr etc.) musst du die Cleanup-Funktion an jeder Stelle wo du einen bestimmten Datentyp brauchst immer wiederholen.



  • vielen Dank für die Tipps. 😃

    Wahrscheinlich ist es wirklich sinnvoller die Variablen nur dort zu deklarieren/initialisieren, wo sie erst benötigt werden. Bisher war ich es immer anders gewohnt. Ich hatte es vor einiger Zeit mal aus einem Coding Guideline übernommen. Ich wollte diesen Stil in meiner jetzigen Programmierung nicht mehr ändern, da es bei ca 35K Programmzeilen schon einen gewissen Aufwand darstellt, dies global anzupassen (ich möchte gerne einen konsistenten Quellcode haben).

    Dank eurer Hilfe konnte ich aber schon einiges verbessern 🙂

    Zum Thema RAII muss ich mich wohl noch etwas besser belesen. Ich halte es schon für wichtig (insbesondere bei Verwendung von exceptions). Viele größere Code-Beispiele verwirren mich noch etwas, aber das wird schon.



  • SeppJ schrieb:

    SBond schrieb:

    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    

    Warum eigentlich static? Ohne funktioniert es scheinbar auch.

    Weil ich mir dachte, dass der wahrscheinlichste Fall ist, dass dies global in einer Implementierungsdatei stehen wird, in der du das brauchst. Und da dann eben static, um das Ding nicht unnötig überall sichtbar zu haben.

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    

    Kann man das auch in zwei Zeilen zerlegen?

    Sollte machbar sein, wenn man ein paar Hürden überspringt, aber wieso das nicht sinnvoll ist, hat manni66 erklärt. Du willst ja gerade von dieser Art der Programmierung weg.

    Hat alles super funktioniert! Vielen Dank 😃

    nochmal zur Trennung in 2 Zeilen. Die Lösung war ja doch recht einfach:

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(nullptr, TYPE_closer);
    //......
    meineVariable.reset(eineOpenSslFunktion(...));
    

    (ist nur noch mla zur Ergänzung)



  • Ich wollte diesen Stil in meiner jetzigen Programmierung nicht mehr ändern, da es bei ca 35K Programmzeilen schon einen gewissen Aufwand darstellt, dies global anzupassen (ich möchte gerne einen konsistenten Quellcode haben).

    Kann ich irgendwo verstehen. Andrerseits sollte man genau diesen Grund immer hinterfragen. Speziell wenn klar ist wie die bessere Variante aussieht, und es nicht 100 verschiedene Arten es zu machen gibt, sondern nur die schlechte und die bessere, sollte man nochmal drüber nachdenken ob es nicht doch besser wäre mal etwas "Uneinheitlichkeit" zu akzeptieren.
    Neue Funktionen nach dem besseren Muster zu stricken. Und alle Teile wo man mehr als nur 1-2 Zeilen anpasst bei der nächsten Änderung einfach umstellt. Ich mache das in einem Programm das ich "geerbt" habe schon die letzten paar Jahre, und es hat mir das Leben durchaus deutlich erleichtert.
    Einige Teile sind immer noch ein unübersichtlicher furchtbarer sauhaufen, aber andere sind dadurch mittlerweile fast schon "gut wartbar" geworden.

    Kommt aber natürlich auch drauf an wie lange du das Programm noch weiterentwickeln und/oder pflegen musst. Wenn du das in ein paar Monaten nicht mehr anfassen musst, dann ist es vielleicht wirklich OK es einfach mal so zu lassen.



  • Ich denke die Wartbarkeit und der Lebenszyklus der Software spielen schon eine Rolle 🙂
    Naja... wenn meine Software wirklich gut sein sollte, dann bräuchte ich noch 1-2 Optimierungsdurchläufe. Ich sehe es an den Bereichen, bei denen ich gerade mit C++ angefangen hatte. Zumindest habe ich in den letzten Monaten einige Fortschritte erziehlt.

    Nochmal kurz zu den Tipp mit der eigenen Klasse. Da ist natürlich was dran. Momentan sehe ich zwei Möglichkeiten meine bestehenen Funktionen zu verbessern:

    1. mit unique_ptr/shared_ptr:

    Header:

    static auto X509_closer = [](X509* t) {X509_free(t);};
    

    Cpp:

    void meineFunktion ()
    {
    	unique_ptr<X509, decltype(X509_closer)> myNewCert(X509_new(), X509_closer);
    	// ...irgendwelcher code ...
    }
    

    2. mit eigener Klasse:

    Header:

    class X509Guard
    {
    public:
    	X509Guard(X509 *x509)
    	{
    		m_x509 = x509;
    	}
    	~X509Guard()
    	{
    		free();
    	}
    	void free()
    	{
    		if (m_x509 != nullptr)
    		{
    			X509_free(m_x509);
    		}
    	}
    
    	X509 *get()
    	{
    		return m_x509;
    	}
    
    	void reset(X509 *x509)
    	{
    		free();
    		m_x509 = x509;
    	}
    
    	X509 *release()
    	{
    		X509 *tmp = m_x509;
    		m_x509 = nullptr;
    		return tmp;
    	}
    
    private:
    	X509 *m_x509;
    };
    

    Cpp:

    void meineFunktion ()
    {
    	X509Guard myNewCert(X509_new());
    	// ...irgendwelcher code ...
    }
    

    Könnt ihr eine Empfehlung geben, welche Variante die bessere ist? Die eigene Klasse hat zumindest den Vorteil, dass diese besser zu lesen ist. Allerdings ist funktionalität im Vergleich zu unique_ptr geringer bzw. ich müsste diese nachimplementieren. Außerdem müsste ich wohl für jeden Datentyp eine Klasse erstellen, da einige Typen verschiedene deleter haben (z.B. von der Anzahl der Parameter). ...ein Template würde wohl nicht so ohne weiteres gehen.

    Oder kann man irgendwie beides Kombinieren? Also ein unique_ptr den ich auch genau so verwende, aber die Initialisierung wie bei meinem 'X509Guard' mache (d.h. die Angabe des deleters wird irgendwie ausgelagert)? ...möglicherwise ginge es über Makros. Aber selbst wenn, wäre es bestimmt keine gute Lösung.

    In erster Linie würde ich zu Variante 1 tendieren, da diese etwas weniger Aufwand darstellt und weniger Code benötigt wird. Mein Problem sind Ausdrücke wie diese hier (Code-Auszug):

    unique_ptr<ASN1_OCTET_STRING, decltype(ASN1_STRING_delete)> subKeyId(nullptr, ASN1_STRING_delete);
    subKeyId.reset(static_cast<ASN1_OCTET_STRING*>(X509_get_ext_d2i(x509 ,nid, nullptr, nullptr)));
    

    Das ist schon in zwei Zeilen aufgebrochen und sieht dennoch unübersichtlich aus. So etwas ist in meinem Code nur 2-3 mal vorhanden, aber ich denke immer, das geht doch bestimmt übersichtlicher.



  • Mach doch ein Typedef.



  • Ich persönlich bevorzuge die Variante mit der eigenen kapselnden Klasse.

    Du erhöhst damit ein wenig die Abstraktion und machst dem durch den Code sozusagen eher klar, was deine Entität macht als wenn du sie nur in einen unique_ptr einkapselst. Und du erreichst etwas Flexibilität für später, falls/wenn du mal deine Interna austauschst, sprich OpenSSL::X509 durch ClosedSSL::KeyStuff ersetzen willst.



  • @Techel: beim Typedef weiß ich gar nicht so recht was ich da machen könnte. Kannst du ein kurzes Beispiel geben, wie so etwas aussehen könnte?

    @Skym0sh0: Wäre es sinnvoll ein Klasse zu erstellen, die gleich mehrere Datentypen verwaltet? Was mich ein wenig von der Klasse abschreckt, ist meine aktuelle Vorstellung davon, für jeden Datentyp eine Klasse zu erstellen. Sollte ich später mal Änderungen machen (z.B. erweitern), dann müsste ich jede Klasse anpassen.

    Es gibt ja dieses 'typeid(meineVariable).name()', das den Typ einer Variable identifiziert. Eventuell könnte man irgendwie den gelieferten Pointer (über den Konstruktor) in ein void* packen und den Datentyp als std::string speichern. Beim Löschen wird dann geschaut, welcher Datantyp es ist. ...oder das ist völliger Blödsinn. ...ist so eine Idee



  • Na zum Beispiel soetwas:

    typedef unique_ptr<ASN1_OCTET_STRING, decltype(ASN1_STRING_delete)> X509Guard
    

    Da hast du dann im Grunde das gleiche wie wenn du eine neue Klasse erstellst.

    Deine letzte Idee ist tatsächlich Blödsinn, das fügt nur unnötigen Overhead hinzu 😉



  • ach mensch natürlich 🙄

    was würde ich nur ohne euch machen. Wenn ihr meine Nachbarn wärt, dann würde ich euch zum Grillen und eine Flasche Bier einladen 😃



  • Ich verwende bei sowas z.T. auch einfach shared_ptr .
    Der Vorteil bei shared_ptr ist dass der Typ des Deleters sich nicht im Smart-Pointer-Typ niederschlägt.
    Damit kann man dann schön Factory-Funktionen machen.
    Die Factory-Funktionen kann man dann verwenden ohne sich darum kümmern zu müssen wie das Objekt jetzt wirklich gelöscht wird.
    Ist vielleicht nicht die "schönste" Lösung (weil man shared-Ownership auch dort ermöglicht wo man es gar nicht braucht, und natürlich den shared_ptr Overhead dadruch auch unnötig mitschleppt). Aber für viele Anwendungen vollkommen ausreichend.

    Ansonsten, wenn dein Deleter Default-Constructible ist, dann kannst du natürlich auch einen unique_ptr typedef verwenden. Dann musst du auch nichts unnötig wiederholen. Durch den typedef vermeidest du den Deleter-Typ überall zu wiederholen und dadurch dass er Default-Constructible ist vermeidest du die Konstruktor-Parameter für den Deleter überall zu wiederholen.



  • Ja stimmt 🙂

    Somit hätte ich wohl folgende Varianten:

    static auto X509_deleter = [](X509* t) {X509_free(t);};
    typedef std::unique_ptr<X509, decltype(X509_deleter)> X509_Guard;
    
    // unique_ptr ohne typedef
    std::unique_ptr<X509, decltype(X509_deleter)> myVar1(nullptr, X509_deleter);
    
    // unique_ptr mit typedef
    X509_Guard myVar2(nullptr, X509_deleter);
    
    // shared_ptr
    std::shared_ptr<X509> myVar3(nullptr, X509_free);
    

    Noch mal eine Frage am Rande. Kann ich sowas problemlos machen?:

    std::shared_ptr<X509> meineFunktion()
    {
        std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);
    
        return myVar1.release();
    }
    
    std::shared_ptr<X509> meineFunktion()
    {
        std::shared_ptr<X509> myVar1(X509_new(), X509_free);
    
        // Variante 1
        return myVar1.release();
    
        // Variante 2
        return myVar1;
    }
    

    ... für den Fall, dass ich einen Pointer teilen will
    (habe es gerade noch nicht getestet)



  • SBond schrieb:

    Noch mal eine Frage am Rande. Kann ich sowas problemlos machen?:

    std::shared_ptr<X509> meineFunktion()
    {
        std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);
    
        return myVar1.release();
    }
    

    Nein.
    Problem #1 Der Deleter wird nicht automatisch mit übernommen. Du musst ihn beim Erzeugen des shared_ptr nochmal angeben.
    Problem #2 Die Konstruktoren von shared_ptr die einen T* nehmen sind explicit .

    So sollte es gehen

    std::shared_ptr<X509> meineFunktion()
    {
        std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);
    
        return std::shared_ptr<X509>(myVar1.release(), myVar1.get_deleter());
    // Oder auch einfach
        return move(myVar1);
    }
    

    SBond schrieb:

    std::shared_ptr<X509> meineFunktion()
    {
        std::shared_ptr<X509> myVar1(X509_new(), X509_free);
    
        // Variante 1
        return myVar1.release();
    
        // Variante 2
        return myVar1;
    }
    

    Variante 1: Nein. shared_ptr hat keine Release Funktion. Darf auch keine haben.

    Variante 2: Ja, das geht.



  • Ich möchte aber nochmal auf die Sache mit default konstruierbaren Deletern hinweisen:

    struct X509_deleter
    {
        void operator()(X509* p)
        {
            if (p)
                X509_free(p);
        }
    };
    
    typedef std::unique_ptr<X509, X509_deleter> X509_Guard;
    
    void Foo()
    {
        X509_Guard g(X509_new());
        // ...
    }
    

    Ist mMn. viel angenehmer zu verwenden.
    Ich mag auf jeden Fall keinen Code wo ich jedes mal wo ich ein Objekt erstelle noch unnötigerweise mit angeben muss wie es gelöscht werden soll.


Log in to reply