Trinitycore: [3.3.5] Crash map.ccp > Map::UpdatePlayerZoneStats

[dikkedeur]

Description:

Core crashes

Current behaviour:

Core crashes, based on crashlog when core updates zone stats while no player is in the zone anymore ?

crashreport: https://gist.github.com/dikkedeur/7a87753762f22e4605d04241bb6268ab

Expected behaviour:

Not crashing please

Steps to reproduce the problem:

No idea what caused it.

Branch(es):

3.3.5.

TC rev. hash/commit:

TrinityCore rev. a1a12ec 2017-09-05 15:33:44 +0200 (3.3.5 branch) (Unix, Debug, Static)

TDB version: TDB 335.63

Operating system: ubuntu server 16.04 LTS

[Raydor]

I found a way to reproduce this:

1. Create a PlayerScript with OnUpdateZone trigger
2. In that OnUpdateZone, insert code player->TeleportTo to another map
3. Insert a player in that zone (for example before doing this script)
4. From another player and in another map, use .app on the player in that zone
5. Watch how the server crashes

I also seen this (but couldn't reproduce easily) when just moving a little bit above Quel'Danas isle, in the fatigue zone there is a place with area and zone <unknown>, it crashed just by entering and leaving that area but not always (it's hard cause I was also playing with .gm on and .gm off).

[dikkedeur]

wauw, neat find then.. i was totally clueless how this was happening.

[Tauriella]

Its not realy fixed!
Example:

Use
.tele Orgrimmar
.tele Ratchet
Crash

[Keader]

@Treeston

[Tauriella]

The same Crash happens when you use the Zepelin from Durotar to Stranglethorn Vale

[Treeston]

@Tauriella did this repro also work before dbabcfb, or is this a regression?

[Tauriella]

I have not tested it before
I currently use TrinityCore rev. 401fd7008acf 2017-09-09 22:03:13 +0200 (335a branch)

[Keader]

@Treeston with last rev, shows in console:

C:\Users\Keader\Documents\Trinity\src\server\game\Maps\Map.cpp:733 in Map::UpdatePlayerZoneStats ASSERTION FAILED:
oldZoneCount A player left zone 4494 (went to 3520) - but there were no players in the zone!

when i did: .tele blacktemple

[ghost]

The assert is still crashing it https://hastebin.com/epuqujijok.hs

This might be related: 2017-09-25_09:55:49 LFGPlayerScript::OnMapChanged, Player Marijukiuka (178833) is in LFG dungeon map but does not have a valid group! Teleporting to homebind.

Last thing to show before the assert ticked in.

@ https://github.com/TrinityCore/TrinityCore/commit/cb9b9f7247c57ee0eed42b063b827799166ce20a

@Treeston

BTW, why do we need an assert there specifically? Wouldn't this kinda work out too?
https://gist.github.com/Nyeriah/422d265bbbd1b30fda8d16ae200ed875

If the issue can be solved in some other way without losses I think it's worth going for it instead of an assert implementation.

[n4ndo]

today i also have this crash:

/Sources/src/server/game/Maps/Map.cpp:732 in UpdatePlayerZoneStats ASSERTION FAILED:
  oldZoneCount A player left zone 1584 (went to 4395) - but there were no players in the zone!

[Treeston]

Removing asserts is never a correct crashfix. Asserts exist for a reason.

[ghost]

Asserts are there to be used at edge cases though. Not to prevent a simple variable overflow from being treated. Having the server crash here or not would have provided you the same results tracking the issue.

My understanding of how asserts were used by TC is that they should be used where a crash would prevent even worse outcomes from happening. Like compromising spawns, corrupting data, causing a crash elsewhere. This is just a variable overflow.

[Treeston]

Just a variable overflow that would then cause the spawn scaling system to go haywire and start respawning mobs at ludicrous rates, yes.

[ghost]

A variable overflow that would then cause the spawn scaling system to go haywire and start respawning mobs as soon at excessive rates.

That is what would indeed happen if it wasn't addressed. I think you are missing the point. You can prevent the overflow from happening by just checking if the zone is empty already before lowering the player count, as in the gist I provided above.

[Shauren]

But that means there is a critical error in player count tracking - assert worthy!

[Treeston]

Precisely what @Shauren said. This assert being thrown means we are screwing up somewhere and our player counts are no longer consistent (<-> trustworthy).

[n4ndo]

Well, i just want to comment, that this is crashing a lot, like 1-2 per hour right now.

[dikkedeur]

for what it is worth, i have a crashlog.

https://gist.github.com/dikkedeur/9fc5c3c34abf282db6623447e4941095

rev: TrinityCore rev. 62f7debffdb0 2017-09-10 17:24:03 +0200 (3.3.5 branch) (Unix, Debug, Static)
TDB 335.63

i dont know if more is needed, or that you guys have all information already.. if so then i wont post more crashlogs.

gr.

[Treeston]

More crashlogs are always helpful, would be even better if you could isolate behaviors that seem related.

[dikkedeur]

@Treeston i have my server running 24/7 so im not always online, only a couple of old friends play on it say 15 people.. and they dont have a clue when it happens and why ^^ 👍 so when i ask what happened they say dunno it stopped working lol.. so i cant say how to reproduce it.. basically it just happens..

[Treeston]

It's got to be some strange edge case behavior related to people teleporting between maps. The last crashlog you posted is someone being teleported from inside Utgarde Keep (the instance) to the Alliance inn in Dalaran in some funky way that doesn't properly reset the last zone information on the player.

Try asking your players to remember if they were being teleported/about to be teleported when the crash happens. If yes, any kind of details about the circumstances would be really helpful - I don't see any code path that'd be responsible for MAP_INVALID_ZONE not being set right now...

[Treeston]

PS: The one before that was a teleport from Blackrock Depths, once again to Dalaran.

I wonder if there's some weirdness going on with instance maps...

[Raydor]

@Treeston https://github.com/TrinityCore/TrinityCore/issues/20289#issuecomment-328293197 did you test this? It keeps crashes with that way, maybe that helps you to find a fix with a good way to test it.

[Treeston]

Hm @Raydor that crash was supposed to be fixed in 46657cc. I'll take a look - do you have a simple diff/repro to cause it?

[ghost]

Have you tried logging to an expired instance ID? I wanted to test that when I got home yesterday but I forgot. When you try logging while inside an expired instance, before you are "fully" added to the instance map, you get ported to hearthstone location. Maybe that has something to do with it?

Remind that the last line I saw at console before the assert was this:

2017-09-25_09:55:49 LFGPlayerScript::OnMapChanged, Player Marijukiuka (178833) is in LFG dungeon map but does not have a valid group! Teleporting to homebind.

[Treeston]

Yeah, something like that was my suspicion too, but the code paths don't seem to back it up. Not saying it's impossible.

[Jildor]

Confirm, this happen when you enter in Instance and automatic teleport to another place

By example I add code in vault of archavon for teleport out of instance the player when enter in the instance and the fortress not is of his faction

and here is the crashlog: https://gist.github.com/Jildor/25841726bd06da13156ffe4eb9e9b989

@Treeston with that info is it useful? 😄

[Treeston]

got a diff for that code you added? would be amazing for reliable repro

[n4ndo]

At least 5 crashes per day.

• https://pastebin.com/UFMAvYEs
• https://pastebin.com/Hp3tMTyA

[Treeston]

Alright, I've had it with this bug. Making some time now.