Problem with removing unique_ptr from vector

Question

I am struggling with removing a unique_ptr from vector. I have a vector:

std::vector<std::unique_ptr<Agent>>;

which I am filling with certain number of Agents. I do it in the following way:

In class constructor list:

agentsVec(std::accumulate(agentsQuantity.begin(), agentsQuantity.end(), 0, 
                           [](int value, const QuantityMap::value_type& p) { return value + p.second; }))

And in a function responsible for generating agents:

auto agentVecIter = agentsVec.begin();
    std::for_each(agentsQuantity.begin(), agentsQuantity.end(), [this, &agentVecIter](const QuantityMap::value_type& p)
    {
        std::generate_n(agentVecIter, p.second, [this, &p]()
        {
            auto agent = factory.createAgent(p.first);
            changeAgentOnLattice(generatePosition(), agent->getID());
            return agent;
        });
        std::advance(agentVecIter, p.second);
    });

agentsQuantity is a map which denotes number of agents of certain type. Agent factory returns std::unique_ptr;

During the program there can be a need to delete some Agent from vector, based on it's ID (Agent's ID is then returned to freeID's stack).

Kill agent:

std::experimental::erase_if(agentsVec, [this, &positionID](auto const& agent) { 
    auto agentID = agent->getID();
    if (positionID == agentID) {
        Utils::addIDToStack(agentID);
        return true;
    }
    return false;
});

To test that, I added 5 agents which will be "killed". In about 40% of cases I got:

Debug Assertion Failed! Vector iterator not dereferencable

What am I doing wrong?

EDIT: More code for clarification. Lattice of agents (which contains agentsVec) is created in Environment class. Environment then step into infinite loop and iterates through all positions in lattice and check if something exists there. If yes -> it moves the agent and updates its health:

Environment::Health update:

for (int i = 0; i < latticeSize; i++) {
    for (int j = 0; j < latticeSize; j++) {
        if (lattice->getAgentID(Position(i, j))) {
            Agent* currentAgent = lattice->getAgentInstance(Position(i, j));
            currentAgent->updateHealth();
            if (currentAgent->getAgentType() == Enums::AgentType::Predator && currentAgent->getHealth() <= -10) {
                lattice->killAgent(Position(i, j));
            }
        }
    }
}

Lattice::Move agent:

const auto originRow = Utils::BoundaryCondition(origin.first, latticeSize);
    const auto originCol = Utils::BoundaryCondition(origin.second, latticeSize);
    const auto destRow = Utils::BoundaryCondition(destination.first, latticeSize);
    const auto destCol = Utils::BoundaryCondition(destination.second, latticeSize);

    if (getAgentID(origin) == static_cast<int>(Enums::AgentType::Field)) {
        Logger::getInstance().Log("lattice", "\tCannot move - there is no moving creature on origin position");
        return false;
    }

    if (getAgentID(destination) != static_cast<int>(Enums::AgentType::Field)) {
        Logger::getInstance().Log("lattice", "\tCannot move - destination position is occupied");
        return false;
    }

    latticeMap[destRow][destCol] = static_cast<int>(getAgentID(origin));
    latticeMap[originRow][originCol] = static_cast<int>(Enums::AgentType::Field);
    Logger::getInstance().Log("lattice", "\tMoved succesfully");

    return true;

Agent getter (used in Environment to get Agent pointer from Lattice::agentsVec)

Agent* Lattice::getAgentInstance(Position position) {
    int ID = getAgentID(position);
    auto it = std::find_if(std::execution::par, agentsVec.begin(), agentsVec.end(),
        [=](std::unique_ptr<Agent>& agent) { return agent->getID() == ID; });
    if (it != agentsVec.end()) {
        return it->get();
    }
    return nullptr;
}

And AgentType getter:

Enums::AgentType Lattice::checkAgentType(int ID)
{
    auto it = std::find_if(std::execution::par, agentsVec.begin(), agentsVec.end(), 
    [=](std::unique_ptr<Agent>& agent)  { return agent->getID() == ID; });

    if(it != agentsVec.end()) {
        return it->get()->getAgentType();
    }
    return Enums::AgentType::Unknown;
}

Do you erase elements from a vector you iterate over? And can you please try to create a [mcve] to show us? And where in your code do you get the assertion failure (you did run in a debugger to catch it?)? — Some programmer dude, Jun 08 '19 at 21:45
@Someprogrammerdude, I based my erase loop on documentation and tutorials, is there a problem I erase element from current-iterated vector. The assertion is shown in Agent class, on getID() method - debugger says it is a nullptr. I will try to add more code you can just copy-paste as soon as I will have access to program code — Michael S., Jun 08 '19 at 21:52
Erasing an element from a vector invalidates all the iterators (and pointers) to the following elements, including the `end` iterator. — Some programmer dude, Jun 08 '19 at 21:56
@Someprogrammerdude, isn't it that erase-remove idiom first move items which will be erased to the end of the vector and after that deletes them? What about this: https://www.fluentcpp.com/2018/09/18/how-to-remove-pointers-from-a-vector-in-cpp/ and https://en.cppreference.com/w/cpp/experimental/vector/erase_if — Michael S., Jun 08 '19 at 22:06
[`std::experimental::erase_if`](https://en.cppreference.com/w/cpp/experimental/vector/erase_if) does the right thing, using the erase-remove idiom. But we don't know what else you're doing when you call `erase_if`. Are you having a loop of your own that you then call `erase_if` in? It's really important that you create a [mcve] to show us that replicates the behavior (if possible) for us to be able to properly help you. We don'y even know if the code you show is relevant for the assertion you get (you *have* used a debugger to catch it?). — Some programmer dude, Jun 08 '19 at 22:11
@Someprogrammerdude, I added everything what's neccessary in that particular use-case. Using debbuger with call stack at exception throw, it breaks on getID method from Agent (it's just an inline getter of int agentID). But it is called from separate thread, which is responsible for graphical lattice rendering (on checkAgentType method). Is the problem caused that Environment tries to remove agent in the same time as GUI tries to iterate over a agentVec? — Michael S., Jun 08 '19 at 23:18
@MichaelS. If you access the vector from 2 threads, then you might get errors if you don't use a `std::mutex` or some other lock to synchronize access to shared data. I have already seen that problem with multithreaded code written by someone who clearly lack knowledge about data race and how to prevent them. — Phil1970, Jun 09 '19 at 01:22

Problem with removing unique_ptr from vector

0 Answers0