Segmentation fault beim Deleten eines Pointers



  • Hallo,
    ich habe eine Klasse StateMachine, die alle States in einem
    std::vector< State* > activeStates speichert. (Ich weiß ich soll smart Pointer benutzen aber...) Wenn ich einen State löschen möchte, gehe ich so vor:

    void StateMachine::popState(State* state)
    {
        if (std::find(activeStates.begin(), activeStates.end(), state) != activeStates.end())
        {
            delete state;
            state = nullptr;
            activeStates.erase(std::find(activeStates.begin(), activeStates.end(), nullptr));
        }
    }
    

    Ich rufe die Methode von der update Methode eines States aus
    StateMachine::popState(this);

    Wenn ich Zeile 7 oder Zeile 5 ausklammere, gibt es immer noch diesen Fehler, es gibt also nicht nur einen Fehler.

    Zur zeit der Übergabe ist *State definitiv valid und wurde nirgends deleted.

    Was ist das Problem?

    Danke für alle Antworten und Liebe Grüße,

    Daniel


  • Mod

    @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    (Ich weiß ich soll smart Pointer benutzen aber...)
    ...
    Was ist das Problem?

    Du weißt, warum das falsch ist, und machst es aber trotzdem so. Da gibt es kein aber. Das eine ist falsch, das andere richtig.



  • @SeppJ Es gibt doch auch einen Weg ohne smart Pointer. Und zum erweitern des Verständnisses für Memory-Management sind raw Pointer doch nicht schlecht



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    @SeppJ Es gibt doch auch einen Weg ohne smart Pointer. Und zum erweitern des Verständnisses für Memory-Management sind raw Pointer doch nicht schlecht

    Schon. Aber es ist extrem schwierig, das 100% korrekt hinzubekommen (auch unter dem Stichwort "Exception safety").

    In deinem Code solltest du einfach das Ergebnis von find speichern. Wenn es != end, dann kannst du einfach den iterator erasen (nachdem du deleted hast) und brauchst nicht ein 2. Mal zu suchen.

    Was ist state denn für ein Objekt? Willst du wirklich State* verwalten? Wie groß ist dein State? Brauchst du überhaupt Zeiger für den State?



  • @wob Okay danke, das werd ich versuchen. Die größe von des States hängt aber von der Implementierung ab, weil es eine virtuelle Klasse ist. Allerdings sind die erbenden Klassen immer ziemlich groß. Die Verantwortung für alle States sollten schon in der StateMachine sein.



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    activeStates.erase(std::find(activeStates.begin(), activeStates.end(), nullptr));

    Das soll jetzt was löschen?



  • @manni66 Den State, glaube ich. Ich hab das hier gefunden.



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    Den State, glaube ich

    glauben ist schlecht, Wissen ist besser 😉

    Damit löscht du den ersten nullptr-Eintrag.
    Mit state = nullptr; hast du aber nur die lokale Variable geändert. Du hast also keinen nullptr in der Collection und wirst somit auch nichts löschen.



  • @wob Achso, wie sollte man stattdessen einen raw pointer aus einem container löschen und entfernen?



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    @wob Achso, wie sollte man stattdessen einen raw pointer aus einem container löschen und entfernen?

    schrieb ich doch oben schon.

    auto it = find...
    if (it != end...) {
        delete *it;
        states.erase(it);
    }
    

    Aber wirklich, delete von Hand ist hässlich. Ohne raw pointer kannst du dir das gesamte if-Geraffel sparen.



  • @wob sagte in Segmentation fault beim Deleten eines Pointers:

    In deinem Code solltest du einfach das Ergebnis von find speichern. Wenn es != end, dann kannst du einfach den iterator erasen (nachdem du deleted hast) und brauchst nicht ein 2. Mal zu suchen.



  • @wob Ah achso; Ich habe das jetzt implementiert:

        auto it = std::find(activeStates.begin(), activeStates.end(), state);
        if (it != activeStates.end())
        {
            delete *it;
            activeStates.erase(it);
        }
    

    Allerdings erzeugt die delete Anweisung einen Core Dump. Das Element existiert ja, sonst würde der Block ja nicht ausgeführt werden. Und die Adresse muss ja valid sein, immerhin wurde sie direkt von dem Objekt übergeben. Der State wurde im StateMachine Konstruktor so erstellt:

    IntroState* const currentState = new IntroState();
    pushState(currentState);
    

    IntroState erbt von State und currentState ist nur eine lokale Variable



  • @daniel

    Allerdings erzeugt die delete Anweisung einen Core Dump.

    Das liegt nicht am gezeigten Code. Die Basisklasse hat einen virtuellen Destruktor?

    Das Element existiert ja, sonst würde der Block ja nicht ausgeführt werden

    Nein, die Adresse existiert im vector.



  • @daniel Zeig' bitte mal ein minimales kompilierbares Beispiel mit dem sich das Verhalten reproduzieren lässt.

    @wob sagte in Segmentation fault beim Deleten eines Pointers:

    Schon. Aber es ist extrem schwierig, das 100% korrekt hinzubekommen (auch unter dem Stichwort "Exception safety").

    Das. Weil wenn Du das dann wirklich exception-safe hast, hast Du einen Smartpointer nachprogrammiert.



  • @manni66 Ich glaube der Fehler liegt darin, dass im Mainloop, welcher über alle State Pointer iteriert, nachdem der Pointer deleted und im Vector entfernt wurde, trotzdem noch die draw() Methode aufgerufen wird. Das Objekt sollte zu dem Zeitpunkt jedoch nicht mehr existieren.

            for (State* state : activeStates)
            {
                if (state->isUpdating())
                {
                    state->handleEvents();
                
                    state->update();
                }
                
                if (state->isDrawing())
                {
                    state->draw();
                }
            }
    

    Allerdings erzeugt if (state->isDrawing()) keinen Fehler, was komisch ist, weil der State Pointer ja nicht mehr existieren sollte. Außerdem wird der Destruktor des States nicht aufgerufen, wie ich sehe. Warum?



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    Ich glaube der Fehler liegt darin, dass im Mainloop, welcher über alle State Pointer iteriert, nachdem der Pointer deleted und im Vector entfernt wurde, trotzdem noch die draw() Methode aufgerufen wird.

    Wie kommst du zu diesem Schluss? Läuft das Löschen und die gezeigte Loop in separaten Threads?

    Ansonsten sind deine Fragen nur per Glaskugel zu beantworten (d.h. der Fehler liegt anderswo).



  • @wob Wenn ich diese Zeile auskommentiere, gibt es keinen Fehler.



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    Ich glaube der Fehler liegt darin, dass im Mainloop, welcher über alle State Pointer iteriert, nachdem der Pointer deleted und im Vector entfernt wurde, trotzdem noch die draw() Methode aufgerufen wird.

    Das geht nur dann, wenn der Vector ein anderer ist als der, aus dem gelöscht wurde.



  • @daniel sagte in Segmentation fault beim Deleten eines Pointers:

    Allerdings erzeugt if (state->isDrawing()) keinen Fehler, was komisch ist, weil der State Pointer ja nicht mehr existieren sollte

    UB (undefined behavior) ist eben genau undefiniert.



  • @manni66 Was passiert denn, wenn ich innerhalb eines for (auto& : states) ein Element lösche, dieses aber noch im Rest der Schleife verwendet wird? Warum erzeugt das keinen Error? Und wie kann es sein, dass der Destruktor des States nicht aufgerufen wird, wenn der Pointer dazu gelöscht wird? Wenn ich jetzt stattdessen einen Smart Pointer nutzen würde, wie würde das ungefähr aussehen? Und würde das nicht zu dem gleichen UB führen, weil der Smart Pointer ja auch nicht mehr existier?


Log in to reply