'std::bad_alloc' when trying to delete priority_queue elements C++

Question

When returning a ship to the port, speed becomes 0.0 and user inputs shield and fuel.

–If fuel is 0.0, the ship gets destroyed

–Ships still in the priority_queue take 10 shield damage and lose 15 fuel

–If shield or fuel become less than 0.0, the ship gets destroyed

Trying to implement these instructions for my final project. The ships are pointer types and they are in a priority queue named 'battlefield'. The ships also exist in a list of pointers called 'port'. I'm trying to destroy the ships that receive lethal damage but when I try to show them, the Qt program crashes and I get bad_alloc error. This is the last thing I have to do for my project :(

Important code blocks from various files:

I already tried to delete the ships from the port, also tried directly deleting them from the port but the priority_queue gets messed up.

class Civilization {
string name;
int x;
int y;
list<Villager> villagers;
list<Ship*> port;
priority_queue<Ship*, vector<Ship*>, Ship::comp> battle;
}


void Civilization::damageShips()
{
priority_queue<Ship*, vector<Ship*>, Ship::comp> copy = battle;
Ship *s = battle.top();
s->setSpeed(0.0);

while(!copy.empty()) {
    Ship *s = copy.top();
    s->setShield(s->getShield() - 10);
    s->setFuel(s->getFuel() - 15);
    copy.pop();
}


priority_queue<Ship*, vector<Ship*>, Ship::comp> temp;

while(!copy.empty()) {
    Ship *s = copy.top();
    string id = s->getId();

    if (s->getShield() > 0 && s->getFuel() > 0) {
        temp.push(s);
    } else
        deleteShip(id);
    copy.pop();
}


battle = temp;
battle.pop();
}

void battlefielddisplay::setCivilization(Civilization *civilizaition)
{
size_t size = civilizaition->battlefieldSize();
ui->battlefield_table->setRowCount(int(size));

Civilization &c = *civilizaition;

priority_queue<Ship*, vector<Ship*>, Ship::comp> copy = c.getBattlefield();

int cnt = 0;
while(!copy.empty()) {
    Ship *s = copy.top();

    QString id = QString::fromStdString(s->getId());
    QString fuel = QString::number(s->getFuel());
    QString speed = QString::number(s->getSpeed());
    QString shield = QString::number(s->getShield());
    QString warriors = QString::number(s->size());

    QTableWidgetItem *idItem = new QTableWidgetItem(id);
    QTableWidgetItem *fuelItem = new QTableWidgetItem(fuel);
    QTableWidgetItem *speedItem = new QTableWidgetItem(speed);
    QTableWidgetItem *shieldItem = new QTableWidgetItem(shield);
    QTableWidgetItem *warriorsItem = new QTableWidgetItem(warriors);

    ui->battlefield_table->setItem(cnt, 0, idItem);
    ui->battlefield_table->setItem(cnt, 1, fuelItem);
    ui->battlefield_table->setItem(cnt, 2, speedItem);
    ui->battlefield_table->setItem(cnt, 3, shieldItem);
    ui->battlefield_table->setItem(cnt, 4, warriorsItem);

    cnt++;

    copy.pop();
}

}

void MainWindow::on_battle_remove_ship_clicked()
{
if (flag) {
    Civilization* c = videogame.searchCivilization(ui->civilization_search_input->text().toStdString());

    double shield = ui->shield_battle_remove->value();
    double fuel = ui->fuel_battle_remove->value();

    Ship *s = c->getBattleShip();
    s->setSpeed(0.0);
    s->setShield(shield);
    s->setFuel(fuel);

    c->damageShips();

    qDebug() << "[✔]" << "Removed ship from battlefield";

} else
    QMessageBox::information(this, "Error", "Civilization not found");
}

bool Civilization::deleteShip(string &id)
{
bool found = false;
for(size_t i(0); i < shipSize(); ++i) {
    auto it = port.begin();
    advance(it, i);
    auto x = *it;
    if (x->getId() == id) {
        port.erase(it);
        delete x;
        --i;
        found = true;
    }
}
return found;
}

In the `deleteShip` you delete the objects but don't remove the pointers from the `port`. What would happen next time when you try to access it? — Dmitry Kuzminov, May 20 '19 at 03:02
Copying priority queues many times - what's the reason for that? — Dmitry Kuzminov, May 20 '19 at 03:04
How do you remove the pointers? I'm still learning about them. — Uriel Guzmán, May 20 '19 at 03:04
I'm making copies because when I want to show what's inside the PQ I need to remove them one by one and I don't want the original PQ to be cleared. — Uriel Guzmán, May 20 '19 at 03:05
References and iterators to the erased elements are invalidated. — n. m. could be an AI, May 20 '19 at 03:10
@UrielGuzmán There is no need to write a loop like that to find an item in a container, call `delete` and then erase it. Whether or not it has bugs, it is error-prone to write code that way. [See this example, using your code](http://coliru.stacked-crooked.com/a/637b31945a35e088). Partition the item(s) to delete, delete them, then erase them. — PaulMcKenzie, May 20 '19 at 03:30

score 0 · Answer 1 · answered May 20 '19 at 03:17

The main problem I see is that you delete the objects without removing the pointers from the container. You are iterating the same container multiple times and trying to access the deleted objects.

An additional problem is that you have multiple copies of the same queue so even removing the pointer from the main container may cause problems.

Try to reconsider the algorithm paying special attention to the life time of the objects. For example you may have a lazy deletion: instead of deleting just mark the objects as those that shall be deleted later. You may have a cleanup at the end of your function.

'std::bad_alloc' when trying to delete priority_queue elements C++

1 Answers1