Sign in to follow this  
KazenoZ

Vectors in copy constructor causes heap corruption.

Recommended Posts

KazenoZ    152
Hello,

I'm feeling that this is a very stupid problem on my part, but I just could not for the life of me figure it out in over a week now =\

I have a class called Event, and that has a member vector<string> commandList. Now, when I call the copy constructor, I want the commandList from the input to be copied to the new class.

So far I tried various ways:
[code]Event::Event(const Event& it){
this->commandList = it.commandList;
}[/code]
[code]Event::Event(const Event& it){
//Both with and without this->commandList.reserve(it.commandList.size());
for(unsigned int i = 0; i < it.commandList; ++i){
this->commandList.push_back(it.commandList[i]);
}
}[/code]
[code]Event::Event(const Event& it){
this->commandList.resize(it.commandList.size());
for(unsigned int i = 0; i < it.commandList; ++i){
this->commandList[i] = it.commandList[i];
}
}[/code]

I tried using substrings, inserts, assigns and so many other ways, I even tried making a new string vector inside the ctor, just to check if the problem is with the source or target, but that works, the problem seems to be with the target vector, to weird thing is that it will start copying the members, but it'll give the error somewhere midway along the loop, if I had 39 objects, it will always crash at 28, for example, even though the capacity is already set to 39...

Whatever I tried, it will ALWAYS end up in the heap corruption error message
[quote][size="1"]HEAP[GameEngine.exe]: HEAP: Free Heap block 3f9350 modified at 3f9d64 after it was freed

Windows has triggered a breakpoint in GameEngine.exe.

This may be due to a corruption of the heap, which indicates a bug in GameEngine.exe or any of the DLLs it has loaded.

This may also be due to the user pressing F12 while GameEngine.exe has focus.

The output window may have more diagnostic information.

HEAP[GameEngine.exe]: HEAP: Free Heap block 3f9d68 modified at 3fb118 after it was freed

Windows has triggered a breakpoint in GameEngine.exe.

This may be due to a corruption of the heap, which indicates a bug in GameEngine.exe or any of the DLLs it has loaded.

This may also be due to the user pressing F12 while GameEngine.exe has focus.

The output window may have more diagnostic information.

[/size][/quote]

I'm just lost for ideas by this point, and I hope that someone can help me shed light on this =\

Share this post


Link to post
Share on other sites
SriLumpa    198
Maybe the problem is not in the class Event, but in the usage you make of it (you try to use an invalid event).

Without more code, we can't tell. You should show the full Event class, and also a minimalist usage that reproduces the problem.

Share this post


Link to post
Share on other sites
KazenoZ    152
There's nothing much else in the Event class to view though, it's all just a bunch of integer and boolean members, and they all get copied fine in the constructor, it's just the vector that causes problems.

I couldn't manage to reproduce the problem in a minimal program, as I don't even know where to start trying to =\

But in the program this goes in, this is usually the last frame in the stack before the ctor is called:
[code]
//...more value setting comes before this point
evC.y = it.y;
evC.name = "exEvent";
evC.solid = 2;

evList.push_back(evC);
++EVENT_COUNT;

it.cmdCount = 0;
++it.evCount;
[/code]

evC and it are both Events, evList is a vector of Events.
The stack will point to ++it.evCount, saying that it is uninitiallized, though looking at the evC that drew values from it, and the copy constructor in the next stack frame, the source values are fine, and the crash still seems to come from the string vector... Again, I'm not really sure about what happens here...

Share this post


Link to post
Share on other sites
Slavik81    360
Your thread title states that the copy construction causes heap corruption, but I don't think that's true. More likely, it reveals previously existing heap corruption as it makes a new allocation. The code you posted for the copy constructor is fine as far as I can tell (though it is unnecessary; the default copy constructor is probably fine).

Share this post


Link to post
Share on other sites
KazenoZ    152
Right... so... where do I look now, actually? What kind of operations do you reckon could cause this? Debugging seems to suggest the push_back method, but I just don't know, the values are valid before it and then corrupt after use, I don't really know where to look now...

Edit:
I tried using the console to make sure about the corruption around the push_back method, and it does seem to crash inside it, the call stack shows:
[code] GameEngine.exe!std::_Construct<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::basic_string<char,std::char_traits<char>,std::allocator<char> > >(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="", const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Val="Number: (zIndex2` add` set` 1)0") Line 52 + 0x36 bytes C++
GameEngine.exe!std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >::construct(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="", const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Val="Number: (zIndex2` add` set` 1)0") Line 155 + 0xd bytes C++
GameEngine.exe!std::_Uninit_copy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Dest="", std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Al={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}) Line 131 + 0x10 bytes C++
GameEngine.exe!stdext::unchecked_uninitialized_copy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,std::basic_string<char,std::char_traits<char>,std::allocator<char> > *,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Dest="", std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > & _Al={...}) Line 822 + 0x55 bytes C++
GameEngine.exe!std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >::_Ucopy<std::basic_string<char,std::char_traits<char>,std::allocator<char> > *>(std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _First="Number: (zIndex2` add` set` 1)0", std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Last=<Bad Ptr>, std::basic_string<char,std::char_traits<char>,std::allocator<char> > * _Ptr="") Line 1141 + 0x18 bytes C++
GameEngine.exe!std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >::operator=(const std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > > & _Right=[3]("Number: (zIndex2` add` set` 1)0","CreateEv: (zIndex2` ev` 7` Zombee` static` 2` 2` 16` 16` 8` Zombee)0","Wait: (set` 10000)0")) Line 595 + 0x1d bytes C++
> GameEngine.exe!Event::Event(const Event & it={...}) Line 647 C++
GameEngine.exe!std::_Construct<Event,Event>(Event * _Ptr=0x00ca73f0, const Event & _Val={...}) Line 52 + 0x36 bytes C++
GameEngine.exe!std::allocator<Event>::construct(Event * _Ptr=0x00ca73f0, const Event & _Val={...}) Line 155 + 0xd bytes C++
GameEngine.exe!std::_Uninit_copy<Event *,Event *,std::allocator<Event> >(Event * _First=0x003faae0, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca73f0, std::allocator<Event> & _Al={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}, std::_Nonscalar_ptr_iterator_tag __formal={...}) Line 131 + 0x10 bytes C++
GameEngine.exe!stdext::unchecked_uninitialized_copy<Event *,Event *,std::allocator<Event> >(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}) Line 822 + 0x55 bytes C++
GameEngine.exe!std::_Uninit_move<Event *,Event *,std::allocator<Event>,std::_Undefined_move_tag>(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}, std::_Undefined_move_tag __formal={...}, std::_Undefined_move_tag __formal={...}) Line 207 + 0x15 bytes C++
GameEngine.exe!stdext::_Unchecked_uninitialized_move<Event *,Event *,std::allocator<Event> >(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Dest=0x00ca6838, std::allocator<Event> & _Al={...}) Line 864 + 0x51 bytes C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::_Umove<Event *>(Event * _First=0x003f9f28, Event * _Last=0x003fb0bc, Event * _Ptr=0x00ca6838) Line 1148 + 0x18 bytes C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::_Insert_n(std::_Vector_const_iterator<Event,std::allocator<Event> > _Where={index=-1414812757 name=<Bad Ptr> pic=0x003fc950 ...}, unsigned int _Count=1, const Event & _Val={...}) Line 1182 C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::insert(std::_Vector_const_iterator<Event,std::allocator<Event> > _Where={index=-1414812757 name=<Bad Ptr> pic=0x003fc950 ...}, const Event & _Val={...}) Line 878 C++
GameEngine.exe!std::vector<Event,std::allocator<Event> >::push_back(const Event & _Val={...}) Line 824 C++
GameEngine.exe!eventResolver(std::vector<Event,std::allocator<Event> > & evList=[15]({index=1 name="" pic=0xbaadf00d ...},{index=6 name="" pic=0xbaadf00d ...},{index=5 name="" pic=0xbaadf00d ...},{index=6001 name="" pic=0x01ec4ae0 ...},{index=5001 name="" pic=0x027b0008 ...},{index=3 name="" pic=0xbaadf00d ...},{index=2 name="" pic=0xbaadf00d ...},{index=3001 name="" pic=0x028f4008 ...},{index=1001 name="" pic=0x01ec4bf0 ...},{index=1001 name="" pic=0x023a0008 ...},{index= na,...), std::vector<Event,std::allocator<Event> > & gevList=[3]({index=-858993460 name="-1" pic=0xcccccccc ...},{index=-858993460 name="Zombee" pic=0xcccccccc ...},{index=-858993460 name="Damage" pic=0xcccccccc ...}), Event & it={...}, Chara & ch={...}, SDL_Surface * screen=0x003b6328, Project & p={...}, std::vector<vPoint,std::allocator<vPoint> > & pList=[1]({x=512 y=736 h=-858993460 ...}), std::vector<vNum,std::allocator<vNum> > & numList=[6]({x=1001.0000 name="zIndex" },{x=2001.0000 name="zIndex2" },{x=3001.0000 name="zIndex3" },{x=4001.0000 name="zIndex4" },{x=5001.0000 name="zIndex5" },{x=6001.0000 name="zIndex6" }), std::vector<vText,std::allocator<vText> > & strList=[0](), std::vector<vSwitch,std::allocator<vSwitch> > & sList=[2]({x=false name="Attacking" },{x=false name="Hit" }), std::vector<vPic,std::allocator<vPic> > & picList=[1]({x=0x003bff78 basePic=0x003bff78 name="Zombee" ...}), std::vector<vSound,std::allocator<vSound> > & sndList=[0](), int scale=2) Line 9527 C++
GameEngine.exe!Map::Play(Project project={...}, Chara ch={...}, std::basic_string<char,std::char_traits<char>,std::allocator<char> > data="~Properties{Name: [Zombees]Path: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\Zombees\Zombees.gpj]}~Settings{Font: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\font.ttf]TBox: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\messageBox.png]Hero: [D:\Mor'sC++\Phantasya Projects\Copy of Zombees\\Zombees\Kyou Sword.PNG]Width: [24]Height: [32]Frames: [4]ScreenW: [800]ScreenH:" ) Line 1027 + 0x76 bytes C++
GameEngine.exe!SDL_main(int argc=2, char * * argv=0x003f37d8) Line 124 C++
GameEngine.exe!_main() + 0xf5 bytes C
GameEngine.exe!__tmainCRTStartup() Line 586 + 0x17 bytes C
[/code]

Lots of frames regarding the vectors and their operations, and the 2 frames that matter are the Event's copy constructor which points to the command list, like I stated above, and the EventResolver that points to the push_back method .


Edit 2:
Digging a little into the stack brought me this:
The frame above the eventResolver is push_back's code:
[code]else
insert(end(), _Val);
}[/code]
Which calls the frame above it, insert, with the value to be put at the end of the vector, so far so good
This is the insert code from the stack:
[code]iterator insert(const_iterator _Where, const _Ty& _Val)
{ // insert _Val at _Where
size_type _Off = size() == 0 ? 0 : _Where - begin();
_Insert_n(_Where, (size_type)1, _Val);
return (begin() + _Off);
}[/code]
Now, _Val still has the value that I need to push back, but the _Where iterator apparently points to an uninitialized Event, so that's probably where it starts going wrong. But why is that?

Share this post


Link to post
Share on other sites
Aardvajk    13207
Default copy constructor will be fine for a class containing a std::vector of std::strings, no need to implement your own. What happens if you omit your own copy constructor completely?

Please show the entire event class - this is probably a violation of the rule of three somewhere.

Share this post


Link to post
Share on other sites
KazenoZ    152
Basically, I need an explicit copy constructor because I need to condition some of the members.
I.E[code]
if(!it.noImg){
this->pic = it.pic;
this->basePic = it.basePic;
}
[/code]
And not for the vector, but as to my understanding, don't I need to explicitly assign all members in case I do use a cctor?

Also, doesn't the Rule of Three state that if I have an explicit ctor, dtor or cctor, I need to make ones for the other 2 as well? In which case, I need to have a basic constructor to intialize my values correctly, so that would mean I need both a copy constructor and a destructor defined as well.
Either way, I have all 3 in the class =\

As par your question about removing the copy constructor, that doesn't do much good, the only difference it does is that now it doesn't show me the stack when it crashes =\

Share this post


Link to post
Share on other sites
Slavik81    360
Do you mean to shallow copy that picture? How is it's lifecycle managed? If you delete it in the destructor you'd be in for a world of hurt.

Share this post


Link to post
Share on other sites
SriLumpa    198
That class looks like a [url="http://sourcemaking.com/antipatterns/the-blob"]blob[/url] ! Maybe fixing this will lead you to finding the problem (or to making it disappear). What is an Event by the way ? it seems to combine the attributes of an actual event, of an entity, and more ...

There is still the possibility that you are *using* the class incorrectly (ie: the problem is outside the class, like using a deleted Event). How about showing eventResolver ?

Share this post


Link to post
Share on other sites
Aardvajk    13207
[quote name='KazenoZ' timestamp='1313332444' post='4848974']
Also, doesn't the Rule of Three state that if I have an explicit ctor, dtor or cctor, I need to make ones for the other 2 as well? In which case, I need to have a basic constructor to intialize my values correctly, so that would mean I need both a copy constructor and a destructor defined as well.[/quote]

No. Rule of three is that if you need any one of the following - copy constructor, destructor or assignment operator - you probably need all three. Constructors themselves are not really relevant to rule of three.

Could you show the body of your destructor for event? I agree with Slavik81 - shallow copy of the SDL_Image is the first thing I'd question.

Share this post


Link to post
Share on other sites
KazenoZ    152
[code]Event::~Event(){
this->commandList.clear();
this->moveList.clear();
if(!this->noImg && this->pic != NULL){
this->pic = NULL;
this->basePic = NULL;
this->noImg = true;
}
}[/code]

The pictures are managed in a picture list object, and each time one is needed it is shallow copied to the appropriate SDL_Surface*, the surface is only freed when the list's destructor is called when the pictures aren't needed anymore, and the respective SDL_Surface*es destruct by being set a NULL, so other pictures using the pointer remain intact.
Is that bad? I went to this way because deep copying still kept giving me crashes when freeing the surface every time an Event is destructed.

@Lumpa, I'm not sure it's really a blob, wouldn't collision checks and movement methods normally go inside the moving object's class? All the members relate to nothing but the Event itself, so I don't see how the class takes over the entire program either.
And yes, the Event is basically all of what you said, it's basically an object in the game that the user defines whether it is just a set of rules, just a static object, an interactable object or NPC or anything else.
It might've been better to go with inheritance, but the purpose was to leave it as open as possible to user editing, and not developer editing, so I figured it would be best not to right now. And just until now it seemed to work well.


I also don't think that I'm trying to operate on a deleted event, since while debugging, when the crash occurs, the whole vector's members are intact, and the problem doesn't even happen when messing with the Event vector, but with the string vector inside the Event vector's members, and even goes halfway in there.

I'd show the eventResolver, but it's a pretty big function that translates the strings and redirects to other functions, it'd be kind of messy to post. The sortEvents function also causes that crash, and is a lot smaller and comprehendable, I'll post that:
[code]//This sorts the events and the Hero's alignment accordingly
void sortEvents(vector<Event> &events, Chara &Hero)
{
//If a new event has been added, or if the Hero moved, the events
//will be sorted, this is to save memory on sorting on every
//pass of the game loop.

//An extra event is added to represent the Hero's
//location.
Event ev;
ev.name = "HERO";
ev.solid = Hero.solid;
ev.y = Hero.y;
events.push_back(ev);
++EVENT_COUNT;

//Sorts the events
sort(events.begin(), events.end(), compareEvents);

//This now searches where the Hero was put in position
//and passes that info to the Hero's drawPos variable
//to be used in the drawing sequence.
for(unsigned int i = 0; i < EVENT_COUNT; i++){
if(events[i].name == "HERO"){
Hero.drawPos = i;
events.erase(events.begin()+i);
--EVENT_COUNT;
break;
}
}
}[/code]

compareEvents compares the events by their solidity and y values for priority on drawing later, the player's character can't be sorted by this way, and so I needed to enter a proxy for it.

Just for the sake of being clear, the basic constructor on the Event class defines the pic and basePic members as NULL at default and clears the commandList and moveList vectors.

Share this post


Link to post
Share on other sites
SriLumpa    198
Sorry no solution, but a few things that you might clarify:

What you are doing in Event destructor is useless, because a destructed Event will not be used again, so all these manipulations that don't release things are not necessary.

Is there a relation between EVENT_COUNT and the size of the event vector given in parameter ? There should be, or your for loop would be dangerous, but then why don't you just use events.size() ?

[quote]I also don't think that I'm trying to operate on a deleted event, since while debugging, when the crash occurs, the whole vector's members are intact, and the problem doesn't even happen when messing with the Event vector, but with the string vector inside the Event vector's members, and even goes halfway in there.[/quote]
Going as far as halfway does not prove much. If you're accessing deleted or corrupted memory, anything can happen. And if you use a delete/corrupted Event, it's precisely *inside* the Event that things are bad.


PS: I still don't see what you call and "event", functionally. For me, an event is something that occurs once. Even if you don't want to break things into many classes, a bit of separation wouldn't hurt, imho.

Edit: another idea: I don't know if a bad comparison function can corrupt a vector while sorting (it can do bad things to map keys at least), but what is compareEvents ? are you sure it is a "strict weak ordering" ?

Edit2: it seems that [url="http://stackoverflow.com/questions/5314672/c-stdsort-using-already-destroyed-object-with-custom-predicate"]it could happen[/url]. So, show us compareEvents :)

Share this post


Link to post
Share on other sites
KazenoZ    152
I know that about the destructor, I only defined one after this error began, since the call stack pointed to it, so I created my own to see what in it crashes, that didn't really help though.

EVENT_COUNT is a global variable storing the events.size() value, when I was trying to optimize it turned out that calling events.size() many times per game loop slowed me down alot, so instead I'm using a simple integer that stores it outside, and it seems to work better, though I'm not sure why the call to size() took that much effort from the machine, shouldn't it be linear complexity?

Event is probably the wrong name to go by, I just called it that automatically by habit from my old days with RPG Maker, it's supposed to be somewhat like that.

This is compareEvents' code:
[code] bool compareEvents(Event a, Event b){
if(a.solid == b.solid){
return a.y < b.y;
}
static const int lookup[] = {1, 0, 2, 1};
return lookup[a.solid] < lookup[b.solid];
}[/code]

I'm pretty sure that's strict weak.
lookup's order is what it is to comply with solid's definition:
[quote]//Solidity of the event
//0 = Solid, 1 = Passes under, 2 = Passes over, 3 = Halfsolid.
int solid;[/quote]

Share this post


Link to post
Share on other sites
SriLumpa    198
vector::size() complexity is constant, so you'd have to call it a lot to notice performance problems (a suspicious lot).

Sorry I'm out of ideas. You should really strip this down, if not for your readers (including yourself), this could help providing a minimal reproducing case.

Share this post


Link to post
Share on other sites
rip-off    10979
Are you sure that all your events have valid "solid" values. Consider adding an assert in your ccomparison function to ensure you aren't stepping outside the bounds of the array:
[code]
template<class T, size_t N>
size_t array_size(T (&)[N])
{
return N;
}

bool compareEvents(const Event &a, const Event &b)
{
if(a.solid == b.solid)
{
return a.y < b.y;
}

static const int lookup[] = {1, 0, 2, 1};

assert(a.solid >= 0 && a.solid < array_length(lookup));
assert(b.solid >= 0 && b.solid < array_length(lookup));

return lookup[a.solid] < lookup[b.solid];
}
[/code]
Note that your event class is rather large, I'd recommend passing it by const reference to functions.

Share this post


Link to post
Share on other sites
KazenoZ    152
I'm pretty sure they have valid values, the solid value assignments are hardcoded, and don't have any incremental locations. I added the assertion anyway, but it doesn't do any good.

Also, I'm not sure how it's supposed to work for me with the const reference, I need the event class to be able to be changed, doesn't const lock it from editing?


I'm lost again by this point... Isn't there anything that might hint you guys at something?

Share this post


Link to post
Share on other sites
SriLumpa    198
[quote name='KazenoZ' timestamp='1313438739' post='4849529']
I'm pretty sure they have valid values, the solid value assignments are hardcoded, and don't have any incremental locations.
[/quote]
It could get a bad value if you implement a copy constructor and forget to copy this field.

[quote name='KazenoZ' timestamp='1313438739' post='4849529']
Also, I'm not sure how it's supposed to work for me with the const reference, I need the event class to be able to be changed, doesn't const lock it from editing?
[/quote]
You can at least easily add this in the comparison method as shown by rip-off.
For other places, if you don't want to refactor ... you can try passing around a pointer or shared_ptr of Event, but I'm not sure adding this complexity is a good idea at this point. You should first fix the problem before you think of optimizing.

Share this post


Link to post
Share on other sites
rip-off    10979
[quote]
Also, I'm not sure how it's supposed to work for me with the const reference, I need the event class to be able to be changed, doesn't const lock it from editing?
[/quote]
Passing a parameter as a const reference doesn't "lock" the original object. Declaring an object const "sortof" does (in standardese) it isn't legal to make changes to an object that was originally declared declared const.

Can you post your EventResolver class header/source, and your Event class source?

Share this post


Link to post
Share on other sites
iMalc    2466
When it comes to a class with say 10 members where you find yourself needing to implement either the copy-constructor, assignment operator, or destructor for justf one of the members in that class, the [b]wrong[/b] thing to do is exactly that.
The right thing to do in that case is to implement a separare 'smart' class that contains just that member and has the required copy-constructor, assignment operator and destructor for just that member. Then you include this 'wrapper' smart-class as a member in your larger class and presto, you're done. Of course when you can use an existing smart-class for that one member instead then even better.


Share this post


Link to post
Share on other sites
KazenoZ    152
I just found the problem, and as always it was just a very simple, stupid, thing I've been missing...

The game didn't crash because of corrupted data, it crashed because of the constant need to resize the vector... just using a reserve function at the creation time of the vector fixed the problem.
This would have been very viable for this system due to the constant growth and no way to determine a good size to reserve, but atleast it's a start, and I can atleast approximate that a user won't need more than a 100...


Thanks for the help guys...

Share this post


Link to post
Share on other sites
iMalc    2466
Congratulations, you probably just hid your bug, and increased your programs memory footprint to boot.
A program doesn't crash just because a vector resizes. It crashes because you've either not followed the rule of three or because you're keeping pointers somewhere to the items within it. It is better to recognise and plan for the fact that a vector can reallocate its internal buffer, than it is to try and prevent that from happening. There are some cases where it is legimate to prevent the reallocating, but this does not appear to be one of them, and many of those cases would involve a resize rather than a reserve.
IMHO a program should probably function identically whether it contains any reserve calls or not, apart from the fact that it may be a tad slower without.

Share this post


Link to post
Share on other sites
Aardvajk    13207
Agree with iMalc. You have hidden the problem, not solved it. The worst that constant resizing of a vector can do is slow your program down, not crash it.

I'd suggest that you make a copy of your codebase, then gradually remove code until you have a minimal example than exhibits the crash. There is still something amiss with your code and if you ignore it now, you will just have an even bigger codebase to debug when, inevitably, the problem resurfaces in the future.

Best case scenario - the problem resurfaces [i]before[/i] you release. :)

Share this post


Link to post
Share on other sites
KazenoZ    152
You guys are right, I didn't know it could not be because of the actual resizing, thank you for that.

I just found the real culprit however.

The process tree that I follow is something like this:
During the main game loop, I process a loop through my current event list, and call the eventResolver on each relevant(Active) event, one of the eventResolver's parameters is a reference to the caller, to update its' location on the commandList when it's done processing.
The problem was that I used to pass the argument to that parameter as evList[i] for the current event, but when the command gets to the part where it changes the list's members, the index would change, and so would the reference that I've been using.

To counter this problem, I changed eventResolver's return type to Event, instead of relying on the reference, and changed the parameter that was originally a parameter by reference to be a parameter by value, saving ahead the index of the current event and putting it back in the list later, to counter the index changing:
[code]if(evList[i].active){
int j = evList[i].index;
//The argument in question is the third argument.
Event ev = eventResolver(evList, gevList, evList[i], ch, screen, project, pList,
numList, strList, sList, picList, sndList, scale);
for(unsigned int k = 0; k < evList.size(); ++k){
if(evList[k].index == j){
evList[k] = ev;
}
}
} [/code]

Thank you all very much for your help.
And I hope that this solution is sufficient, solving the problem altogether and not hiding it again?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this