Crash when checking collisions in Qt6

SGaist

As suggested, run your application through the debugger to see exactly where it crashes.

black_gay

@SGaist debugger shows segmentation fault at line: if(dynamic_cast<Bullet*>(itemsCollideWithShip[i])){
Another way to solve the problem is:
```
QGraphicsItem* item = itemsCollideWithShip.takeAt(i);
delete item;
item = nullptr;

One more is adding i++.
Which suggests, that for some reason a nullpointer cannot be present in QList, though this check "if(itemsCollideWithShip[i]==nullptr) continue;" prevents using nullpointer, so i dont understand why would i need to remove this nullpointer from the QList

SGaist

QList has no problem storing nullptr.

You should not modify the size of container while you are iterating through it in a for loop.

You can build a list of the index to remove and do it after you're done with your for loop.

black_gay

@SGaist The main question is why would i even want to delete nullpointer from QList at this point, as i only wish to delete the colliding item from bullets (a vector of Bullet*)? I iterate through the QList one by one, so order of its elements doesnt change and keeping a nullpointer in QList seemingly does no harm, as i make sure i dont use the nullpointer later .... but the behaviour of the program results with a crash (segmentation fault). In short QList shouldnt be affected by altering bullets vector (even though they store the same elements).

JonB

@black_gay
When you step through/run under the debugger and it segment faults, please post the full stack trace, which you will find in some debugger window.

If it actually segments on dynamic_cast<Bullet*>(itemCollidesWithShip[i]), I believe code for dynamic_cast<> has to actually deference the pointer to examine what is there in order to determine its type. That would mean that pointer is pointing somewhere bad --- either to random/bad memory, or possibly to already-free/deleted memory.

black_gay

@JonB This is full stack trace dump:
1 ```
libstdc++-6!.dynamic_cast 0x7ffa394002f2
2 PlayerDefences::checkCollisions playerdefences.cpp 50 0x7ff7c1886545
3 Player::advance player.cpp 97 0x7ff7c188553c
4 Level1::advance level1.cpp 102 0x7ff7c1884522
5 QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (Level1:: *)()>::call(void (Level1:: *)(), Level1 *, void * *) qobjectdefs_impl.h 152 0x7ff7c188d669
6 QtPrivate::FunctionPointer<void (Level1:: *)()>::call<QtPrivate::List<>, void>(void (Level1:: *)(), Level1 *, void * *) qobjectdefs_impl.h 185 0x7ff7c188e33c
7 QtPrivate::QSlotObject<void (Level1:: *)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase *, QObject *, void * *, bool *) qobjectdefs_impl.h 395 0x7ff7c188d86a
8 QMimeType::filterString() const 0x7ffa30d99357
9 QTimer::timerEvent(QTimerEvent *) 0x7ffa30b28a9a
10 QObject::event(QEvent *) 0x7ffa30b0b3b7
11 QApplicationPrivate::notify_helper(QObject *, QEvent *) 0x7ffa30415946
12 QCoreApplication::sendEvent(QObject *, QEvent *) 0x7ffa30ad6ed8
13 QEventDispatcherWin32Private::sendTimerEvent(int) 0x7ffa30c49f2f
14 QEventDispatcherWin32::event(QEvent *) 0x7ffa30c4f700
15 QApplicationPrivate::notify_helper(QObject *, QEvent *) 0x7ffa30415946
16 QCoreApplication::notifyInternal2(QObject *, QEvent *) 0x7ffa30ad69ed
17 QCoreApplicationPrivate::sendPostedEvents(QObject *, int, QThreadData *) 0x7ffa30ada3a9
18 QWindowsGuiEventDispatcher::sendPostedEvents() 0x7ffa2b4b9e6e
19 QEventDispatcherWin32::processEvents(QFlagsQEventLoop::ProcessEventsFlag) 0x7ffa30c4e9f0
20 QWindowsGuiEventDispatcher::processEvents(QFlagsQEventLoop::ProcessEventsFlag) 0x7ffa2b4b9e55
21 QEventLoop::exec(QFlagsQEventLoop::ProcessEventsFlag) 0x7ffa30ae0fd3
22 QCoreApplication::exec() 0x7ffa30adf8b6
23 main main.cpp 9 0x7ff7c1884c52

this is debugger's dump
http://pastebin.com/RgDwe6R8

hskoglund

Hi, just a guess but iperhaps your QList crashes because it contains duplicates of the same bullet (dynamic_cast<> doesn't like casting deleted poiinters).
And that could explain why the extra c++ line you show that avoids the crash by removing duplicates from the QList works (because then that dynamic_cast<> never sees the duplicates).

JonB

@black_gay
Immediately after delete itemCollidesWithShip[i] ; you fail to go itemCollidesWithShip[i] = nullptr. You have never shown you do, because

QGraphicsItem* item = itemsCollideWithShip.takeAt(i);
delete item;
item = nullptr;

the above does not achieve that.

That means you leave a non-nullptr sitting in the list. Next time this code is called it does not follow the if(itemCollidesWithShip[i]==nullptr) continue route. Instead it checks this pointer, now pointing to garbage, for if(dynamic_cast<Bullet*>(itemCollidesWithShip[i])), and that is liable to go weird/crash.

Adding

itemCollidesWithShip.erase(std::remove(itemCollidesWithShip.begin(), itemCollidesWithShip.end(), itemCollidesWithShip[i]), itemCollidesWithShip.end())

removes that pointer from the list (though you ought not do it inside the for loop), hence no crash.

Isn't this all your situation is?

black_gay

@JonB It also crashes when i set itemsCollideWithShip[i] = nullptr. I modified the code as below, but std::bad_case doesnt show anything. I also set loads of debug messages, and it shows that after "delete item" in previous iteration, when i want to acces next item in the QList (ex. 3rd is deleted, and 4th is accesed) in the next iteration of the loop - it crashes (segmentation fault). Crashes can be avoided only if i add i++ inside the loop, or use remove/erase idiom on QList, or store all the colliding elements in a temporary container and then call delete on each of them. But anyway, i cant get over it that the crash appears - no idea how can it be, that the next element gets corrupted by deleting the previous one in QList. There arent duplicated in QList.

void PlayerDefences::checkCollisions(QGraphicsItem* item)
{
    try {
        QList<QGraphicsItem*> itemsCollideWithShip = item->collidingItems();
        std::vector<Bullet*>* bullets = Level1::getBulletContainer();
        for (int i = 0; i < itemsCollideWithShip.size(); i++) {
            if(itemsCollideWithShip[i]==nullptr) continue;
            if(dynamic_cast<Bullet*>(itemsCollideWithShip[i]) ){
                life-=10;
                setActualLife();
                Game::getScene()->removeItem(itemsCollideWithShip[i]);
               bullets->erase(std::remove(bullets->begin(), bullets->end(), itemsCollideWithShip[i]), bullets->end());
                delete itemsCollideWithShip[i];
                itemsCollideWithShip[i] = nullptr;
            }
        }
    } catch (const std::bad_cast& e) {
        std::cout << e.what() << '\n';
    }
}

SGaist

What about dumping the content of the data structures you are using to see what's inside and if anything changes while you are accessing/modifying it ?