Forgottenserver: Summons crashing

[Mkalo]

Before creating an issue, please ensure:

• [x] This is a bug in the software that resides in this repository, and not a
  support matter (use https://otland.net/forums/support.16/ for support)
• [x] This issue is reproducible without changes to the code in this repository

Steps to reproduce (include any configuration/script required to reproduce)

1. Summon any monster
2. Logout

Expected behaviour

Player logout and summon disappear.

Actual behaviour

Crash

Environment

Latest build of TFS 1.3

Related to #2173 but it is crashing for another reason.

Backtrace:
http://pastebin.com/z6nM8Jsp

[Kamenuvol]

I can't reproduce

[Mkalo]

@Kamenuvol That is weird, it happens all the times and I tested like 10 times.

Downloaded the executable from https://ci.appveyor.com/project/kornholi/forgottenserver/build/1.0.389/job/m46qdj81k943f1f9/artifacts

And using the latest data pack (with the summon spell modifications.)

[infernumx]

Was unable to reproduce, tested with fire elemental and monk using utevo res

[Kamenuvol]

@Mkalo Which Windows are you using?

[dead]

I can reproduce. Compiling in a docker VM.

[Mkalo]

Backtrace:
http://pastebin.com/z6nM8Jsp

[Mkalo]

@Kamenuvol Tested in windows 7 and ubuntu 16.04, it crashes in both.

[vankk]

I can confirm this issue.

Using Windows Seven 64 bits.

[Kamenuvol]

I really don't understand why it's happening for you and not for me using same .exe file from appveyor. What is causing that? Now I want to solve the mystery 🤔

[ranisalt]

Can't reproduce too.

[DSpeichert]

Reproduced on latest Ubuntu with the exact same backtrace as given.

[Mkalo]

@Kamenuvol @ranisalt Did you guys download exactly from https://ci.appveyor.com/project/kornholi/forgottenserver/build/1.0.389/job/m46qdj81k943f1f9/artifacts ?

You know that the latest build in the list https://ci.appveyor.com/project/kornholi/forgottenserver also shows pull requests and you might just have downloaded my fixed version of the pull request. At least that's what happened with @Vulcanx.

[ranisalt]

I don't have Windows, but @dspeichert says he has reproduced on Ubuntu too.

[jo3bingham]

Unless I'm reading @Mkalo's GDB dump incorrectly, the segmentation fault is happening at the line summon->setAttackedCreature(nullptr); in:

for (Creature* summon : summons) {
    summon->setAttackedCreature(nullptr);
    summon->setMaster(nullptr);
}

which implies summon is a nullptr. Which means the summon isn't being removed. Upon making sure the summon is actually removed when necessary (#2191), it wouldn't hurt to do a nullptr check:

for (Creature* summon : summons) {
    if (!summon) {
        continue;
    }
    summon->setAttackedCreature(nullptr);
    summon->setMaster(nullptr);
}

[Kamenuvol]

Please, try to print the summon list size on the destructor before this loop.

[Mkalo]

@jo3bingham summon is never nullptr, delete doesn't make every pointer to that object a nullptr.

[jo3bingham]

@Mkalo A segmentation fault happens when trying to access an invalid memory address, which implies that summon is invalid in some way (usually a nullptr).

[Mkalo]

A wild crash has appeared:
http://pastebin.com/M89EkwYu

My Fix in #2190 has the same problem tho. Fixed.

This happens for another reason poitned out by an user in otland in here

Both crashes were gone when I reverted https://github.com/otland/forgottenserver/commit/5ba1ef3926695237a8da3d969bd99e779a0ae7f1