# C++ read access violation when deleting object

In my game, I've created (likely a very bad) Entity Manager that handles all entities in my game. It will assign random ID's to these entities and then add them to a map:

player = new Player(entityManager);
player->SetPosition(S2D::Vector2(50, 250));


//Adds entites to a list and assigns a random ID to them
{
int ID = rand();
for(int i = 0; i < IDS.size(); i++)
{
if(IDS[i] == ID)
{
ID = rand();
i = 0;
}
else
{
break;
}
}

entity->SetID(ID);
entities.insert(pair<int, Entity*>(ID, entity));
}


Each of these entities has a Update and Render function that is ran by the Entity Manager through it's Update and Render functions which are called by the Game Loop:

void EntityManager::Update(float deltaTime)
{
if(!entities.empty())
{
//Removes entities that need to be deleted
for(int i = 0; i < toDeleteEntities.size(); i++)
{
RemoveEntity(toDeleteEntities[i]);
toDeleteEntities.erase(toDeleteEntities.begin() + i);
}

//Runs the Update function of all entities and adds Entites that need to be deleted to a list
for(const auto entity : entities)
{
if(entity.second->ShouldDestroy())
{
toDeleteEntities.push_back(entity.second);
}

entity.second->Update(deltaTime);
}
}
}

void EntityManager::Render()
{
if(!entities.empty())
{
for(const auto entity : entities)
{
entity.second->Render();
}
}
}


As well as looping through each entity, the Update function also loops through a vector called "toDeleteEntities" which contains entities that are to be deleted. Each Entity has a boolean called "Destroy" and if it's set to true, the Update function in the Entity Manager will add that entity to the vector.

//Runs the Update function of all entities and adds Entites that need to be deleted to a list
for(const auto entity : entities)
{
if(entity.second->ShouldDestroy())
{
toDeleteEntities.push_back(entity.second);
}

entity.second->Update(deltaTime);
}


Then before that for loop, the Update function will run this for loop that will remove each entity from the "toDeleteEntities" vector and then pass it onto another function that will delete it:

//Removes entities that need to be deleted
for(int i = 0; i < toDeleteEntities.size(); i++)
{
RemoveEntity(toDeleteEntities[i]);
toDeleteEntities.erase(toDeleteEntities.begin() + i);
}


That delete function is this:

//Removes an Entity from the Game
void EntityManager::RemoveEntity(Entity* entity)
{
if(entity->GetID() == -572662307)
{
return;
}

entities.erase(entity->GetID());
delete entity;
}


The if Statement is there because of a previous error I had where I believe the for loop would pass the same entity to the delete function twice resulting in it trying to delete a already deleted object. I know it would be better to fix the error but truthfully it's easier to just do that XD.

Anyway hopefully that's enough context, if you need anymore then just ask. So all of this seems to work correctly somewhat with this Gem entity I have, it runs correctly and gets deleted without problems. But I've noticed when I attempt to add new entities and try and delete them, I get this _Pnext error:

And I just can't figure out why. It seems to be coming from this loop here in the xutility class:

Does anyone know why this error is happening?

EDIT Screenshot of the Call Stack:

• It's really important that you use breakpoints and step through your code. You will have many more of these. This isn't really the place for these things. – Sidar Jan 6 '18 at 9:40
• @Sidar Well I know the error is coming from the "delete entity"... I maybe should of mentioned that – codelyoko373 Jan 6 '18 at 17:19
• doesn't matter much tho. You're asking a debug question. And if you insist it's more suited for regular stackoverflow. – Sidar Jan 7 '18 at 13:13

Looks like the issue is your toDeleteEnemies loop. As you remove entries, your list will shrink, but your counter is still going up. So when you get past half way, you're now out of bounds of the array. You can also optimize this either way by just clearing the list after going through each entry, as you are only acting on each item once

Example:

for (auto entity : toDeleteEntities)
RemoveEntity(entity);
toDeleteEntities.clear();


A few other suggestions, Use smart pointers, to avoid having to manually call delete (and end up with other potential memory bugs).

And, avoid hard coded numbers (magic numbers). Define them as constants somewhere and refer to them by name

One other note, if you are adding an item to the toDelete list, you probably don't need to call its update method afterwards

• Wouldn't the list shrinking and the counter still going up give me a out of range error instead? – codelyoko373 Jan 6 '18 at 7:00
• accessing begin() + i is where your error is happening. C++ iterators are pretty 'forgiving' – CobaltHex Jan 6 '18 at 7:00
• So are you saying removing that line of code will fix the error I am having since unfortunately it doesn't fix it? – codelyoko373 Jan 6 '18 at 7:05
• I've updated the post with what I mean – CobaltHex Jan 6 '18 at 8:13
• By the way, you can utilize the call stack window of your debugger to try and figure out the source of the issue – CobaltHex Jan 6 '18 at 8:50