Keskustelu: Ohjelmointikysymykset: C++: Outo kaatuminen

Olen juuri viimeistelemässä peliini StateManageria, joka siis huolehtii siitä pelin eri tiloista (ollaanko pelissä vai valikossa jne.). Törmäsin kuitenkin outoon bugiin. Yritin käyttää kahta vektoria tallentamaan pointtereina pelin tilat, toinen aktiivisille toinen epäaktiivisille tiloille. Toisen vektorin lisääminen saa kuitenkin ohjelman kaatumaan.

StateManager.h:

#ifndef STATEMANAGER_H
#define	STATEMANAGER_H

#include "State.h"
#include <vector>

class StateManager {
public:
    StateManager();
    StateManager(const StateManager& orig);
    virtual ~StateManager();

    void addActiveState(State* activeState);
    void addInactiveState(State* inactiveState);

    void removeActiveState();
    void removeDeadStates();

    State* getActiveState();
private:
    std::vector<State*> deadStates;
    std::vector<State*> states;

};

#endif	/* STATEMANAGER_H */

StateManager.c:

#include "StateManager.h"

StateManager::StateManager() {
}

StateManager::StateManager(const StateManager& orig) {
}

StateManager::~StateManager() {
    std::vector<State*>::iterator iter;

    for(iter = states.begin(); iter != states.end(); ++iter ) {
        delete *iter;
    }
    states.clear();

   for(iter = deadStates.begin(); iter != deadStates.end(); ++iter) {
        delete *iter;
    }
    deadStates.clear();

}

void StateManager::addActiveState(State* newState) {

    states.push_back(newState);
    states.back()->init(this);
}

void StateManager::addInactiveState(State* newState) {
    states.insert(states.begin(), newState);
}

void StateManager::removeActiveState() {
    State* state = states.back();
    deadStates.push_back(state);
    states.pop_back();
}

void StateManager::removeDeadStates() {

    std::vector<State*>::iterator iter;
    for(iter = deadStates.begin(); iter != deadStates.end(); ++iter) {
        delete *iter;
    }
    deadStates.clear();

}

State* StateManager::getActiveState() {
        return states.back();
}

Debuggerin ilmoitus:

Signal received: SIGSEGV (?) with sigcode ? (?)
From process: ?
For program breakout_clone, pid 7 891

(NetBeans sanoo vain, että "Run failed".)

Jos kommentoin toisen näistä riveistä:

std::vector<State*> deadStates:
std::vector<State*> states;

koodi toimii moitteetta (tietenkin ilman toista vektoria). Jos vektorit ovat noin päin, peli ei edes lähde käyntiin vaan kaatuu heti, kun states-vektoria yritetään käyttää (kohdasta en ole vielä ihan varma, huomasin tämän vasta kun olin kirjoittamassa tätä). Jos vektorit laittaa toisin päin, ohjelma kaatuu, kun deadStates vektorista yritetään deletoida jäseniä. Vaikuttaisi siltä, että ensimmäinen vektori vuotaa toisen päälle, mutta en kyllä ymmärrä miksi... Teenkö jotain väärin? Ja tosiaan, tämä kuuluu ohjelmoinnnin harjoitustyöhöni ja se pitäisi saada valmiiksi tänään. :)

EDIT:
Jos deadStates on ennen statesia, ohjelma kaatuu, kun StateManagerilta getActiveState():n kautta pyydettyä pointteria yritetään käyttää.

No tutki nyt sillä debuggerilla, millä rivillä koodi kaatuu, mistä sinne on päädytty ja mitä muuttujat sisältävät.

Tuosta ei käy paljonkaan olennaista ilmi. Mistä State-oliot tulevat? Kai luot ne new-operaattorilla? Missä ne lisätään StateManageriin? Missä kutsut getActiveState-funktiota ensimmäisen kerran, onko silloin varmasti tiloja?

Kannattaisi laittaa ainakin funktioihin removeActiveState ja getActiveState tarkistus, että tiloja yleensä on, ja virhetilanteessa joko heittää poikkeus (esim. std::logic_error) tai edes keskeyttää funktio (jälkimmäisestä siis palauttaa 0).

Jos kopiokonstruktoria ei ole tarkoitus käyttää, tee siitä vaikka private ja heitä sieltäkin jokin poikkeus. Ääritapauksessa voisit jättää jopa toteutuksen pois, jolloin käännöksestä tulisi virhe; tosin jokin kääntäjä voisi mukista tarpeettomankin toteutuksen puuttumisesta.

Lisäsin tarkistuksia:

void StateManager::removeActiveState() {
    if(!states.empty()) {
        State* state = states.back();
        deadStates.push_back(state);
        states.pop_back();
    }
}

void StateManager::removeDeadStates() {
    if(!deadStates.empty()) {
        std::vector<State*>::iterator iter;
        for(iter = deadStates.begin(); iter != deadStates.end(); ++iter) {
            delete *iter;
        }
        deadStates.clear();
    }

}

State* StateManager::getActiveState() {
    if(!states.empty())
        return states.back();
    else
        return 0;
}

State oliot tulevat täältä:

void Game::init(int width, int heigth, int depth, std::string title) {

    window = new sf::RenderWindow(sf::VideoMode(width, heigth, depth), title);
    window->SetFramerateLimit(60);

    //stateManager.addActiveState(new GameState(window));
    //stateManager.addActiveState(new MenuState(window));
    stateManager.addActiveState(new VictoryState(window));
    stateManager.addActiveState(new MenuState(window));
}

void Game::run() {

     while(window->IsOpened()) {

        State* activeState = stateManager.getActiveState();

        if(activeState != 0) {
            activeState->update();
            activeState->handleEvents();
            activeState->draw();
        }
     }

}

Mainissa kutsutaan ensin Game::init() ja sitten Game::run(). getActiveStatea kutsutaan ainoastaan tuossa ja tiloja on varmasti. getActiveState myös palauttaa jotain muuta kuin nollan ja ohjelma kaatuu, kun yritetään kutsua tuota updatea.

Lisäsin tarkistukset noihin funktioihin. Kiirrellä kun vääntää niin niitä tarkituksia tuppaa unohtumaan. ;D

Muuten en olisi koko kopiokonstruktoria toteuttaa, mutta NetBeans tunkee sen sinne automaattisesti, enkä jokaisesta luokasta vain ole jaksanut sitä poistaa. Tein siitä nyt kuitenkin privaten.

Kun deadStates on jälkimmäisenä, ohjelma kaatuu siihen, kun sen jäseniä yritetään poistaa removeDeadStatesissa (ja nimenomaan juuri kohdassa delete *iter). Siinä vaiheessa deadStates-vektorin sisältö on aika outo (ennen yhtäkään poistoa):

_M_end_of_storage 0x0
_M_finish 0x6353f8
_M_start 0x0

Verrattuna statesiin:

_M_end_of_storage 0x114e6b0
_M_finish 0x114e6b0
_M_start 0x114e6a0

Lisäsin removeDeadStatesiin tarkistuksen, mutta deleteä kutsutaan silti, vaikka deadStatesin pitäisi olla tyhjä.

void StateManager::removeDeadStates() {
    if(!deadStates.empty()) {
        std::vector<State*>::iterator iter;
        for(iter = deadStates.begin(); iter != deadStates.end(); ++iter) {
            delete *iter;
        }
        deadStates.clear();
    }

}

Pysäytin debuggauksen tuohon kohtaan ja se sanoo tällaista:

_M_end_of_storage 0x0
_M_finish 0x6353f8
 *_M_finish 0x74756f6b61657242
_M_start 0x0

Tuo *_M_finish vaikuttaisi olevan jokin haamu instanssi, jonka kaikki arvot ovat tyhjiä.

No oma syy, mitäs kikkailet osoittimilla. :) Luultavasti nyt jossain on virheellinen osoitin, jonka kautta onnistut sorkkimaan väärää muistia. Muista aina alustaa osoittimet. Fiksu veto olisi myös korvata suurin osa niistä boost::shared_ptr:llä (tai std::tr1::shared_ptr:llä).

Käännä nyt vielä varmuuden vuoksi koko projekti uudestaan, miten sitten NetBeansilla tapahtuukaan. Jos se ei tunnista tiedostojen riippuvuuksia oikein, voi olla, että eri tiedostoilla on eri käsitys jonkin luokan sisällöstä.

Ei tuosta paljon muuta pysty lyhyiden koodinpätkien perusteella sanomaan. Pysäytä ohjelma muutamassa eri kohdassa ja tarkista nuo arvot. (Jos debuggaustaitoja riittää, voit käskeä myös debuggerin pysäyttää automaattisesti, kun kyseiset muuttujat muuttuvat.)

Funktiossa removeDeadStates tarkistus on turha, koska tyhjä vektori ei tyhjentämisestä miksikään muutu.

Tiesitkös, ettei ole kovin fiksua poistaa objektia, kun jokin pointteri vielä osoittaa siihen? Meinasin viime viikolla ottaa Boostin käyttöön, mutta ajattelin että koko projektin muuntaminen veisi liikaa aikaa tässä vaiheessa. Nyt kyllä ketuttaa. :)

Kävin läpi joka ikisen pointterin ja varmistin, että jokainen asetetaan nollaksi konstruktorissa, jos sille ei kostruktorissa anneta muuta arvoa. Parihan sieltä oli tietenkin unohtunut. Sen jälkeen ohjelma kaatuikin loogisemmassa paikassa ja vian löytäminen oli suhteellisen helppoa. Kiitos avusta.

Niin, ja ei vara venettä kaada vaikka se tarkistus turha onkin. Saattaa siitä debuggauksessa olla hyötyä. :)