Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Very rare server crash on creature events. #2570

Closed
marcinho1994 opened this issue Apr 25, 2019 · 9 comments
Closed

Very rare server crash on creature events. #2570

marcinho1994 opened this issue Apr 25, 2019 · 9 comments
Labels
bug An issue describing unexpected behavior of code
Milestone

Comments

@marcinho1994
Copy link
Contributor

marcinho1994 commented Apr 25, 2019

Steps to reproduce

  1. Create a monster and make it "active". That means letting the monster attack another monster with (SPELLS) *The target monster tested here, have an event onHealthChange.

  2. Now remove the target monster with this code:

local function RemoveCreature(cid)
    local monster = Creature(cid)
    if not monster then
         return false
    end
    monster:remove()
end
  1. Repeat the process a thousand times until you crash.

Expected behaviour

Remove the monster without major problems

Actual behaviour

If the monster is removed at the exact moment it is dropping a spell the server crashes. (Important this was only tested with spells in area).

Environment

Linux - Ubuntu 18

GDB LOGS:

#0  Combat::getCombatArea (centerPos=..., targetPos=..., 
    area=area@entry=0x1605ac0, list=empty std::forward_list) at combat.cpp:88
No locals.
#1  0x000000000042f25e in Combat::CombatFunc(Creature*, Position const&, AreaCombat const*, CombatParams const&, std::function<void (Creature*, Creature*, CombatParams const&, CombatDamage*)>, CombatDamage*) (caster=caster@entry=
    0xe095680, pos=..., area=0x1605ac0, params=..., func=..., 
    data=data@entry=0x7ffff5d49ba0) at combat.cpp:729
        tileList = empty std::forward_list
        spectators = std::unordered_set with 4560616172927782688 elements = {
          [0] = 0xc0310a74d2851e74<error reading variable: Cannot access memory at address 0x2fa833b7401fa83>...}
        rangeX = <optimized out>
        rangeY = <optimized out>
        player = 0x0
        maxX = <optimized out>
        maxY = <optimized out>
#2  0x000000000042f635 in Combat::doCombatHealth (
    caster=caster@entry=0xe095680, position=..., area=<optimized out>, 
    damage=..., params=...) at combat.cpp:869
No locals.
#3  0x0000000000430ce9 in Combat::doCombat (this=0x111d490, caster=0xe095680, 
    position=...) at combat.cpp:835
        damage = {primary = {type = COMBAT_ICEDAMAGE, value = -223}, 
          secondary = {type = COMBAT_NONE, value = 0}, origin = ORIGIN_SPELL, 
          critical = false}
#4  0x00000000005c5744 in CombatSpell::castSpell (this=0x1127c00, 
    creature=0xe095680, target=<optimized out>) at spells.cpp:345
No locals.
#5  0x000000000054541f in Monster::doAttacking (this=0xe095680, interval=1000)
    at monster.cpp:774
        inRange = true
        __for_range = <optimized out>
        updateLook = false
        resetTicks = true
#6  0x0000000000465b7e in Game::checkCreatures (this=0x86e260 <g_game>, 
    index=<optimized out>) at game.cpp:3908
        creature = 0xe095680
        __for_range = std::vector of length 277, capacity 512 = {0x5ff0000, 
          0x5f84c00, 0x5f85400, 0x5f48400, 0x5f4a000, 0x5f4bc00, 0x5ec7000, 
          0x5df6800, 0x5d1d000, 0x5bbcc00, 0x5bbe400, 0x5bbec00, 0x5b9f800, 
          0x5a87400, 0x5a87800, 0x5a4c800, 0x5965c00, 0x5967400, 0x5967800, 
          0x5925000, 0x5925c00, 0x5926c00, 0x5890400, 0x586c400, 0x586f000, 
          0x586f400, 0x5858000, 0x585a400, 0x5848c00, 0x581bc00, 0x57d4400, 
          0x57d4800, 0x57d5000, 0x57aa400, 0x57abc00, 0x5794400, 0x5794c00, 
          0x5795400, 0x5796000, 0x5775400, 0x5776800, 0x5777400, 0x5759400, 
          0x575a400, 0x5740400, 0x5741c00, 0x5743000, 0x5723400, 0x56f4400, 
          0x56f5800, 0x56f7400, 0x56ec000, 0x56ed400, 0x56d4800, 0x56ba800, 
          0x56bb400, 0x56bbc00, 0x56a7000, 0x5658400, 0x5659c00, 0x5628400, 
          0x5628c00, 0x562ac00, 0x5606400, 0x5607800, 0x55e6c00, 0x55ccc00, 
          0x55ce000, 0x55bd000, 0x55bd800, 0x55bf000, 0x55a8000, 0x55a9000, 
          0x55a9800, 0x5579c00, 0x557a400, 0x5569000, 0x5569400, 0x555c800, 
          0x555d400, 0x555e400, 0x555e800, 0x555f000, 0x554c000, 0x554d400, 
          0x5511000, 0x5511800, 0x54f7400, 0x54dc800, 0x54df000, 0x54df800, 
          0x54c4800, 0x5492400, 0x5451400, 0x5451c00, 0x541a400, 0x53fb000, 
          0x53fb400, 0x2815800, 0x2813000, 0x2814800, 0x6d5e940, 0xb8a6000, 
          0x50030c0, 0x4e64000, 0x86b92c0, 0xf365a40, 0xe1ebb00, 0x3089e00, 
          0x4db30c0, 0x8df12c0, 0x8e06940, 0x8b71a40, 0x9762580, 0x4e443c0, 
          0x7e8a1c0, 0x8edd680, 0x8e252c0, 0xb989c00, 0x9e92000, 0x11467e00, 
          0x8d7d2c0, 0x808de00, 0xf70e580, 0x8b4cb40, 0x8568780, 0xad23600, 
          0xd2d9680, 0x4954940, 0x3b71a40, 0x8b4d2c0, 0x922ad00, 0x8692940, 
          0x9e94400, 0x83ad680, 0x7c28b40, 0xf8210c0, 0x8178f00, 0xc06bc00, 
          0x4da0f00, 0x93c3480, 0x8e60b40, 0x8175a40, 0x11820780, 0xc889c00, 
          0x30e9e00, 0x6e7e580, 0xd592d00, 0x6243c00, 0x7e1f840, 0x68ad680, 
          0x11aac940, 0x3b85680, 0x96d21c0, 0x93e6940, 0x90d3c00, 0x922c3c0, 
          0x410e940, 0xaf49600, 0xad22d00, 0xc68e000, 0xdc73c00, 0x10d46b40, 
          0x827f0c0, 0x4f7f480, 0x11456d00, 0x96930c0, 0x6218f00, 0x88cf840, 
          0x40212c0, 0x3192940, 0x9207480, 0x97192c0, 0x64652c0, 0x7c54f00, 
          0xe63db00, 0x5025680, 0x841d680, 0x9008780, 0x698c000, 0x875f840, 
          0xc067480, 0x8b20780, 0x35d7840, 0x7f1f480, 0x9651a40, 0x9214b40, 
          0x971de00, 0x4f94780, 0x765a1c0, 0x6dd6d00, 0x947cf00, 0x7f86d00, 
          0x8e28f00, 0x83f6940, 0xd3403c0, 0x7ee7c00, 0x7caf480, 0x8b51680, 
          0xe3db680...}
        checkCreatureList = std::vector of length 277, capacity 512 = {
          0x5ff0000, 0x5f84c00, 0x5f85400, 0x5f48400, 0x5f4a000, 0x5f4bc00, 
          0x5ec7000, 0x5df6800, 0x5d1d000, 0x5bbcc00, 0x5bbe400, 0x5bbec00, 
          0x5b9f800, 0x5a87400, 0x5a87800, 0x5a4c800, 0x5965c00, 0x5967400, 
          0x5967800, 0x5925000, 0x5925c00, 0x5926c00, 0x5890400, 0x586c400, 
          0x586f000, 0x586f400, 0x5858000, 0x585a400, 0x5848c00, 0x581bc00, 
          0x57d4400, 0x57d4800, 0x57d5000, 0x57aa400, 0x57abc00, 0x5794400, 
          0x5794c00, 0x5795400, 0x5796000, 0x5775400, 0x5776800, 0x5777400, 
          0x5759400, 0x575a400, 0x5740400, 0x5741c00, 0x5743000, 0x5723400, 
          0x56f4400, 0x56f5800, 0x56f7400, 0x56ec000, 0x56ed400, 0x56d4800, 
          0x56ba800, 0x56bb400, 0x56bbc00, 0x56a7000, 0x5658400, 0x5659c00, 
          0x5628400, 0x5628c00, 0x562ac00, 0x5606400, 0x5607800, 0x55e6c00, 
          0x55ccc00, 0x55ce000, 0x55bd000, 0x55bd800, 0x55bf000, 0x55a8000, 
          0x55a9000, 0x55a9800, 0x5579c00, 0x557a400, 0x5569000, 0x5569400, 
          0x555c800, 0x555d400, 0x555e400, 0x555e800, 0x555f000, 0x554c000, 
          0x554d400, 0x5511000, 0x5511800, 0x54f7400, 0x54dc800, 0x54df000, 
          0x54df800, 0x54c4800, 0x5492400, 0x5451400, 0x5451c00, 0x541a400, 
          0x53fb000, 0x53fb400, 0x2815800, 0x2813000, 0x2814800, 0x6d5e940, 
          0xb8a6000, 0x50030c0, 0x4e64000, 0x86b92c0, 0xf365a40, 0xe1ebb00, 
          0x3089e00, 0x4db30c0, 0x8df12c0, 0x8e06940, 0x8b71a40, 0x9762580, 
          0x4e443c0, 0x7e8a1c0, 0x8edd680, 0x8e252c0, 0xb989c00, 0x9e92000, 
          0x11467e00, 0x8d7d2c0, 0x808de00, 0xf70e580, 0x8b4cb40, 0x8568780, 
          0xad23600, 0xd2d9680, 0x4954940, 0x3b71a40, 0x8b4d2c0, 0x922ad00, 
          0x8692940, 0x9e94400, 0x83ad680, 0x7c28b40, 0xf8210c0, 0x8178f00, 
          0xc06bc00, 0x4da0f00, 0x93c3480, 0x8e60b40, 0x8175a40, 0x11820780, 
          0xc889c00, 0x30e9e00, 0x6e7e580, 0xd592d00, 0x6243c00, 0x7e1f840, 
          0x68ad680, 0x11aac940, 0x3b85680, 0x96d21c0, 0x93e6940, 0x90d3c00, 
          0x922c3c0, 0x410e940, 0xaf49600, 0xad22d00, 0xc68e000, 0xdc73c00, 
          0x10d46b40, 0x827f0c0, 0x4f7f480, 0x11456d00, 0x96930c0, 0x6218f00, 
          0x88cf840, 0x40212c0, 0x3192940, 0x9207480, 0x97192c0, 0x64652c0, 
          0x7c54f00, 0xe63db00, 0x5025680, 0x841d680, 0x9008780, 0x698c000, 
          0x875f840, 0xc067480, 0x8b20780, 0x35d7840, 0x7f1f480, 0x9651a40, 
          0x9214b40, 0x971de00, 0x4f94780, 0x765a1c0, 0x6dd6d00, 0x947cf00, 
          0x7f86d00, 0x8e28f00, 0x83f6940, 0xd3403c0, 0x7ee7c00, 0x7caf480, 
          0x8b51680, 0xe3db680...}
#7  0x00000000005cae18 in operator() (this=0xa0f5310)
    at /usr/include/c++/4.8/functional:2471
No locals.
#8  operator() (this=0xa0f5300) at tasks.h:40
No locals.
#9  Dispatcher::threadMain (this=0x86ea60 <g_dispatcher>) at tasks.cpp:60
        task = 0xa0f5300
        taskLockUnique = {_M_device = 0x86ea78 <g_dispatcher+24>, 
          _M_owns = false}
#10 0x00007ffff6d1ca60 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
No symbol table info available.
#11 0x00007ffff6539184 in start_thread (arg=0x7ffff5d4a700)
    at pthread_create.c:312
        __res = <optimized out>
        pd = 0x7ffff5d4a700
        now = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737317742336, 
                -7811287349239907049, 1, 0, 140737317743040, 140737317742336, 
                7811300192288603415, 7811307931975515415}, 
              mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, 
            data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = <optimized out>
        pagesize_m1 = <optimized out>
        sp = <optimized out>
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#12 0x00007ffff626603d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
No locals.
@raymondtfr
Copy link
Contributor

raymondtfr commented Apr 25, 2019

marcinho1994 please use the issue template. Are you using the latest commits from master?

Possibly related to #2558 #2566 #2568.

@EvilHero90

Or even #2554.

@jo3bingham

@marcinho1994
Copy link
Contributor Author

@raymondtfr when i discovered this bug i had checked that the classes and functions were 100% aligned with the master. This was a few months ago. I'll check again. I fixed this bug on my server but it was not an ideal solution. I posted this bug here so that some acceptable solution may come up for everyone who may have come to suffer from it.

@raymondtfr
Copy link
Contributor

As I said in the previous post, you have got to use the issue template so we can make sure this issue is present in this repository and not a side effect of sources that has been modified.

Anyway, is the crash you posted above from months ago like you said?

@marcinho1994
Copy link
Contributor Author

marcinho1994 commented Apr 25, 2019

@raymondtfr
1º Reformulated Template.

2º Mine was fixed months ago with a bad solution.

3º I will check again if this happens with the master repo, i'm downloading this now.

4º This GDB LOG is a recent crash where it happened in a server of my friend. I identified that the log was the same as mine and decided to post here already so that nobody suffers more with this bug.

@EvilHero90
Copy link
Contributor

EvilHero90 commented Apr 25, 2019

probably this:

void Combat::doCombat(Creature* caster, Creature* target) const
{
	//target combat callback function
	if (params.combatType != COMBAT_NONE) {
		CombatDamage damage = getCombatDamage(caster, target);
		if (damage.primary.type != COMBAT_MANADRAIN) {
			doCombatHealth(caster, target, damage, params);
		} else {
			doCombatMana(caster, target, damage, params);
		}
	} else {
		doCombatDefault(caster, target, params);
	}
}

void Combat::doCombat(Creature* caster, const Position& position) const
{
	//area combat callback function
	if (params.combatType != COMBAT_NONE) {
		CombatDamage damage = getCombatDamage(caster, nullptr);
		if (damage.primary.type != COMBAT_MANADRAIN) {
			doCombatHealth(caster, position, area.get(), damage, params);
		} else {
			doCombatMana(caster, position, area.get(), damage, params);
		}
	} else {
		CombatFunc(caster, position, area.get(), params, CombatNullFunc, nullptr);
	}
}

as we do not check if caster is valid before we apply the damage and just push the dangling pointer onwards.

@marcinho1994
Copy link
Contributor Author

marcinho1994 commented Apr 26, 2019

I did more than 2 hours of testing here and discovered new things and discovered the specific situations, I will edit the issue with this informations:

Discovered: It was discovered that for the bug to occur, one must call the "monster: remove ()" within a creature with a registered event (creaturescripts), the tests were done within the "onHealthChange" event.

Basic solution: Do not do it.

*This issue can be closed. Because in my internal investigations, that lasted hours they ended up verifying that it was something not of the source, but of a bad programming in lua. I'm sorry for that!

@marcinho1994
Copy link
Contributor Author

marcinho1994 commented Apr 26, 2019

Hello Guys! I have discovered, after hours and hours of testing. I discovered the error and the "ideal solution" i will post here and you can fix them.

SPELLS.CPP ()

bool CombatSpell::castSpell(Creature* creature, Creature* target)
{
        if (!creature || !target) // added this line
                return false; // added this line
	if (scripted) {
		LuaVariant var;

		if (combat->hasArea()) {
			var.type = VARIANT_POSITION;

			if (needTarget) {
				var.pos = target->getPosition();
			} else if (needDirection) {
				var.pos = Spells::getCasterPosition(creature, creature->getDirection());
			} else {
				var.pos = creature->getPosition();
			}
		} else {
			var.type = VARIANT_NUMBER;
			var.number = target->getID();
		}
		return executeCastSpell(creature, var);
	}

	if (combat->hasArea()) {
		if (needTarget) {
			combat->doCombat(creature, target->getPosition());
		} else {
			return castSpell(creature);
		}
	} else {
		combat->doCombat(creature, target);
	}
	return true;
}

Sorry because i never had used github before, and i'm starting now...
Sorry for my bad english to all, thanks for who had try to help :)
Fix this and will be all fine, this error will never more occurs

@DSpeichert
Copy link
Member

So is this a bug in master or not? Because you say it needs a fix and you close the issue too.

@marcinho1994
Copy link
Contributor Author

Yes, its a bug in master. It occurs in very specific and rare situations where monsters must be in very specific situations as well.
I investigated the situation by whole and had an idea of how to crash. Should I create a PR?

@DSpeichert DSpeichert reopened this May 9, 2019
@Znote Znote added the bug An issue describing unexpected behavior of code label Sep 4, 2019
@Znote Znote added this to the 1.3 milestone Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue describing unexpected behavior of code
Projects
None yet
Development

No branches or pull requests

5 participants