[C++] iterator geeft ptr in debugger

[codeneos]

Om maar gelijk met de deur in huis te vallen, ik zit methet volgende probleem: als ik over een std::map heel loop crasht het programma (het crasht niet structureel op één punt).

Dit is be gerbuikte code:

C++:

    
UpdateClass::Update(std::map<OBJECT_HANDLE, Object *> &m)
{

std::map<OBJECT_HANDLE, Object *> tmp(m);
for(std::map<OBJECT_HANDLE, Object*>::iterator iter=tmp.begin(); iter != tmp.end(); ++iter)
        iter->second->Update(i_diff);

}

Het prgramma is gecompiled onder vs2005 zonder optimalisaties en met debug options.

De visual studio debugger geeft in de locale waarde weer dat iter->second niet bestaat maar iter->ptr->second wel. Het iter->ptr->second bevat dan de laaste object uit de map. De applicatie crasht niet alleen in deze loop maar ook in andere soort gelijke loops, is er een manier om dit te voorkomen.

[rrrandy]

Ik heb niet veel kaas gegeten van c++, maar je doet een ++iter, dus eerst iter ophogen en dan de code uitvoeren. Loop je dan sowieso niet altijd bij de laatste steevast eentje te ver?

[GX]

Nee, die ++iter wordt aan het einde van je loop uitgevoerd.

Zou het komen dat door je update functie de map uitgebreid word, of de map.end van waarde veranderd? (Edit: Ik probeer te zeggen dat je misschien een eindeloze loop maakt)

[ Voor 18% gewijzigd door GX op 07-07-2006 09:10 ]

[codeneos]

Nee de update functie voegt geen nieuwe items toe aan de map. Tevens wordt de map eerst naar een lokale variable gekopieerd. Het zou wel kunnen dat een ander deel van het programma de iter verwijderd maar dit lijkt me ook sterk aangezien ik al geprobeerd heb dit er tussen te zetten en dat hielp niet:

C++:

if(!(iter*))
    continue;

Bij mijn weten is er geen (groot) verschil tussen ++itr en itr++. (in het resultaat bedoel ik)

[.oisyn]

En je weet dus zeker dat iter->second altijd een geldig Object* bevat?
Aan je code lijkt me niets verkeerd namelijk.

[DroogKloot]

Maak je code ten eerste eens wat leesbaarder want ik krijg er koppijn van. C++ scoort al niet zo hoog op dat gebied maar icm. STL wordt het echt een brei, en aangezien debuggen nou eenmaal een stuk makkelijker gaat wanneer je een beetje overzichtelijkheid aanbrengt:

C++:

UpdateClass::Update(std::map<OBJECT_HANDLE, Object*> &m) {

    std::map<OBJECT_HANDLE, Object*> map(m);
    std::map<OBJECT_HANDLE, Object*>::iterator iter = map.begin();
    std::map<OBJECT_HANDLE, Object*>::iterator end = map.end();

    for (; iter != end; ++iter) {
        iter -> second -> Update(i_diff);
    }
}

Als (iter -> second) niet altijd een geldige pointer is (wat niet anders kan gezien het feit dat je programma telkens op verschillende punten crasht) dan moet OF de initialisatie van je map al fout zijn OF een andere update operatie ergens ervoor zorgen dat die values gecorrumpeerd worden. Zou je misschien wat meer context kunnen posten?

[ Voor 23% gewijzigd door DroogKloot op 07-07-2006 12:50 ]

[.oisyn]

DroogKloot schreef op vrijdag 07 juli 2006 @ 12:45:
Maak je code ten eerste eens wat leesbaarder want ik krijg er koppijn van.

Dat valt best mee, ik denk eerder dat jij gewoon niet echt gewend bent aan de standaard iterator for-lussen. Een typedef voor z'n map had wel netter geweest though.

Maar als je het echt gaat hebben over netheid moet je het gewoon zo doen:

C++:

typedef std::map<OBJECT_HANDLE, Object*> ObjectMap;

void map_entry_update(const ObjectMap::value_type & entry, int diff)
{
    entry.second->Update(diff); 
}

void UpdateClass::Update(const ObjectMap & m)
{
    ObjectMap tmp(m);
    std::for_each(tmp.begin(), tmp.end(), std::bind2nd(std::ptr_fun(map_entry_update), i_diff));
}

boost heeft hier overigens wat betere helper classes voor

[ Voor 18% gewijzigd door .oisyn op 07-07-2006 15:39 ]

[schoene]

Ik vraag me trouwens wel af wat hier het nut is van de locale kopie?

[codeneos]

Het is mogelijk dat een andere class de waardes van het object heeft veranderd maar de objecten worden niet verwijderd dus dat zou het probleem niet moeten zijn.

Ik vond de code toch nog behoorlijk net . In de vs ide is hij behoorlijk leesbaar, op got wat minder (ander color coding) .

De waarde van objecten kunnen door de gehele code worden veranderd, het programma maakt gebruikt van meerdere threads d.m.v zthread.

[Soultaker]

Misschien gaat het al bij het maken van de kopie fout? Je synchroniseert niets, dus als tijdens het maken van de kopie de map gewijzigd wordt, kan het best mis gaan (al is de kans daarop bij std::map wel veel kleiner dan bij andere datastructuren).

[codeneos]

Ok, dus direct met &m werken inplaats van een copy te maken onder een nieuw aanduiding (in dit geval tmp). Ik ga het proberen maar volgens mij kan dit weinig uit maken.

Ik ga ook de meer 'cleane' methode van .oisyn proberen, misschien door dat dit een andere manier is van over de std::map heen lopen dat het niet een dergelijke error produceerd.

wat ik nog wel steeds vreemd vind is dat iter ipv second & first nu de variable ptr heeft die de second & first bevatten. Kan iemand hier een verkalring voorgeven of is hier misschien een regel voor wanneer dit soort errors voorkomen en waardoor? (aangezien ik deze error al vaker tegen ben gekomen)

Ook heb ik me afgevraagt of het misschien een compiler error is, het lijkt me eigenlijk sterk...

[Soultaker]

codeneos schreef op vrijdag 07 juli 2006 @ 18:42:
Ok, dus direct met &m werken inplaats van een copy te maken onder een nieuw aanduiding (in dit geval tmp). Ik ga het proberen maar volgens mij kan dit weinig uit maken.

Ik weet niet hoe je die conclusie trekt, maar dat was niet mijn suggestie. Je kunt beter eens naar zinnige synchronisatie kijken als je multithreaded bezig bent.

Ik ga ook de meer 'cleane' methode van .oisyn proberen, misschien door dat dit een andere manier is van over de std::map heen lopen dat het niet een dergelijke error produceerd.

Nope, het is simpelweg een meer high-level manier om op te schrijven wat je nu zelf al doet.

wat ik nog wel steeds vreemd vind is dat iter ipv second & first nu de variable ptr heeft die de second & first bevatten. Kan iemand hier een verkalring voorgeven of is hier misschien een regel voor wanneer dit soort errors voorkomen en waardoor? (aangezien ik deze error al vaker tegen ben gekomen)

Dat is geen fout; dat is de lay-out van die iterator nu eenmaal. De reden dat je in je code it->first kunt (moet!) doen, is dat operator-> overloaded is (en it.ptr sowieso niet toegankelijk is, waarschijnlijk). Ik heb het idee dat je C++ kennis niet van zo'n nivo is dat je dat helemaal kunt volgen, maar ga er maar vanuit dat die standard library objecten allerlei rare implementaties hebben die je als 'gebruiker' normaal gesproken niet te zien krijgt, maar in de debugger dus wel.

Dat is wel wat lastig debuggen, maar dus geen bug, en je zult het ook merken bij andere classes.

Ook heb ik me afgevraagt of het misschien een compiler error is, het lijkt me eigenlijk sterk...

Inderdaad - dat is het laatste waar je aan moet denken.

[MSalters]

.oisyn schreef op vrijdag 07 juli 2006 @ 15:31:
Maar als je het echt gaat hebben over netheid moet je het gewoon zo doen:

C++:

typedef std::map<OBJECT_HANDLE, Object*> ObjectMap;

struct map_entry_update {
  int diff;
  map_entry_update(int diff) : diff(diff) { }
  void operator()(ObjectMap::value_type const& entry) { entry.second->Update(diff); }
};

void UpdateClass::Update(const ObjectMap & m)
{
    ObjectMap tmp(m);
    std::for_each(tmp.begin(), tmp.end(), map_entry_update(i_diff));
}

Sommige bindings zijn te ver gezocht.

[.oisyn]

dat had ik eerst staan, maar quite frankly is dat gewoon meer typwerk . En wat beter is is in dit geval puur subjectief. Aan de struct-methode zou je meer gehad hebben als het mogelijk was geweest die struct lokaal in de functie te definieren, maar helaas kun je de template er dan natuurlijk niet mee instantieren

[ Voor 64% gewijzigd door .oisyn op 07-07-2006 23:19 ]

[Soultaker]

Erm, wake up: beide suggesties zijn een stuk langer dan de code van de TS, moeilijker te schrijven, en naar mijn mening geen steek duidelijker.

[codeneos]

Oke, maar nu heb ik nog steeds niet door hoe/waardoor (oorzaak, dus een globale omschrijnving welke handelinging je zou moeten verichten om deze error te reproduceren) deze error ontstaat . Dat de ptr een interne waarde binnen de iterator had ik al door (heb eerst wat gegoogled voor dat ik de vraag op got heb gepost ).

Maar inderdaad kan zelf ook nog niet zeggen dat ik een c++ expert ben. Desalniettemin is de kans dat het object verwijderd word in één thread en nog aan geroepen wordt door het andere erg klein. Alle objecten worden bewaard in een singelton class via waar ze ook benaderd worden.

Ook dacht ik er aan dat try {..} catch {...} misschien een oplossing zou bieden. Dit zodat de code niet vastloopt en het programma gewoon door kan gaan. Nu ben ik niet erg bekend mat deze manier van error afhandeling in c++ zou het een oplossing bieden?

[.oisyn]

codeneos schreef op zaterdag 08 juli 2006 @ 08:44:
Maar inderdaad kan zelf ook nog niet zeggen dat ik een c++ expert ben. Desalniettemin is de kans dat het object verwijderd word in één thread en nog aan geroepen wordt door het andere erg klein.

Is de kans klein of bestaat ie gewoon niet? Indien dat eerste is het natuurlijk vragen om moeilijkheden, als je je map of de objecten in meerdere threads gebruikt zul je het een en ander moeten synchronizeren

Alle objecten worden bewaard in een singelton class via waar ze ook benaderd worden.

Dat zegt helemaal niets over thread-safety

Ook dacht ik er aan dat try {..} catch {...} misschien een oplossing zou bieden. Dit zodat de code niet vastloopt en het programma gewoon door kan gaan. Nu ben ik niet erg bekend mat deze manier van error afhandeling in c++ zou het een oplossing bieden?

Nee, de bug blijft, je bestrijdt alleen de symptomen.