Server Crash - SnapNewID()

bretonium commented 5 years ago

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000055755bf7531b in dbg_break () at src/base/system.c:87
87              *((volatile unsigned*)0) = 0x0;
[Current thread is 1 (Thread 0x7fa651ef2740 (LWP 11404))]
(gdb) where
#0  0x000055755bf7531b in dbg_break () at src/base/system.c:87
#1  0x000055755bf752e8 in dbg_assert_imp (filename=0x55755c0289f1 "src/engine/server/server.cpp", line=153, test=0, msg=0x55755c0289e8 "id error")
    at src/base/system.c:81
#2  0x000055755bf803bf in CSnapIDPool::NewID (this=0x7fa64d2ed7cc) at src/engine/server/server.cpp:153
#3  0x000055755bf8bb31 in CServer::SnapNewID (this=0x7fa64cc00010) at src/engine/server/server.cpp:2758
#4  0x000055755bfb8cbb in CGameContext::CreateLaserDotEvent (this=0x7fa651ebd010, Pos0=..., Pos1=..., LifeSpan=8) at src/game/server/gamecontext.cpp:246
#5  0x000055755bff4291 in CGrowingExplosion::Tick (this=0x55755e542560) at src/game/server/entities/growingexplosion.cpp:213
#6  0x000055755bfb04ff in CGameWorld::Tick (this=0x7fa651ec0178) at src/game/server/gameworld.cpp:236
#7  0x000055755bfbc5da in CGameContext::OnTick (this=0x7fa651ebd010) at src/game/server/gamecontext.cpp:1031
#8  0x000055755bf89104 in CServer::Run (this=0x7fa64cc00010) at src/engine/server/server.cpp:2213
#9  0x000055755bf8c189 in main (argc=1, argv=0x7ffda8c5e4c8) at src/engine/server/server.cpp:2862

Please note that it happened with my version https://github.com/bretonium/my-infclass-server/tree/test-master (test-master branch)

bretonium commented 5 years ago

I have the core file. Please ping me if you would like to gdb it, i will have to give you access to the server

duralakun commented 5 years ago

gamecontext https://github.com/bretonium/my-infclass-server/blob/test-master/src/game/server/gamecontext.cpp#L246 growingexplosion https://github.com/bretonium/my-infclass-server/blob/test-master/src/game/server/entities/growingexplosion.cpp#L213

I am not sure why it happend, does anyone have an idea? Looks like a scientist mine explosion triggered it? It tried to Snap a new id and then crashed somehow

CServer::SnapNewID() https://github.com/bretonium/my-infclass-server/blob/test-master/src/engine/server/server.cpp#L2758

This dbg_assert got triggered https://github.com/bretonium/my-infclass-server/blob/test-master/src/engine/server/server.cpp#L153

So it tries to get a new id, but the id is -1 and then it crashes. I think in order to debug this we need to understand how the id system works and why the id became -1

I think the two important functions to understand are these ones:

void CSnapIDPool::RemoveFirstTimeout() int CSnapIDPool::NewID()

here you can see them: https://github.com/bretonium/my-infclass-server/blob/test-master/src/engine/server/server.cpp#L127 if you understand them pls write about them here

bretonium commented 5 years ago

I have a naive suggestion. Can it be that we just run out of ids?

duralakun commented 5 years ago

Here MAX_IDS is defined https://github.com/bretonium/my-infclass-server/blob/test-master/src/engine/server/server.h#L21 currently the max is 32768 In order to debug this issue maybe we can log every 30 min how many ids are currently used, if this number grows somehow than maybe there is an id leak? But i dont understand the id system right now so it is hard to talk about it for me

teoman002 commented 5 years ago

SnapNewID works like that.

First you define an instance variable array in scientist-mine.h which provides space for IDs
Now you call the constructor which calls: m_IDs[i] = Server()->SnapNewID();
As long as an entity exists, IDs exist
By calling the destructor of one object, the server does Server()->SnapFreeID(m_IDs[i]);
If you don't call the destructor all the time, IDs become rare.
If no ids are left, server returns -1.

Since the server was running 4 days in series:

not only new but also old features could have an ID leak.

teoman002 commented 5 years ago

TODO:

write markedfordestroy for each weapon entity, to be consistent. invalid

duralakun commented 5 years ago

@teoman002 is this not what is happening already here: https://github.com/bretonium/my-infclass-server/blob/test-master/src/game/server/entities/character.cpp#L3673

duralakun commented 5 years ago

@bretonium pls use this https://github.com/yavl/teeworlds-infclassR/pull/111

bretonium commented 5 years ago

Crashed again:

(gdb) where
#0  0x000056404ea3d31b in dbg_break () at src/base/system.c:87
#1  0x000056404ea3d2e8 in dbg_assert_imp (filename=0x56404eaf09f1 "src/engine/server/server.cpp", line=153, test=0, msg=0x56404eaf09e8 "id error")
    at src/base/system.c:81
#2  0x000056404ea483bf in CSnapIDPool::NewID (this=0x7f4c92f507cc) at src/engine/server/server.cpp:153
#3  0x000056404ea53b31 in CServer::SnapNewID (this=0x7f4c92863010) at src/engine/server/server.cpp:2758
#4  0x000056404ea7a1e2 in CEntity::CEntity (this=0x5640508e29c0, pGameWorld=0x7f4c97b23178, ObjType=0) at src/game/server/entity.cpp:26
#5  0x000056404ea9df79 in CProjectile::CProjectile (this=0x5640508e29c0, pGameWorld=0x7f4c97b23178, Type=1, Owner=2, Pos=..., Dir=..., Span=100, Damage=1, 
    Explosive=false, Force=0, SoundImpact=-1, Weapon=1, TakeDamageMode=0) at src/game/server/entities/projectile.cpp:12
#6  0x000056404eaa5a38 in CCharacter::FireWeapon (this=0x56404ed82b70 <ms_PoolDataCCharacter+3376>) at src/game/server/entities/character.cpp:1031
#7  0x000056404eaa84d6 in CCharacter::HandleWeapons (this=0x56404ed82b70 <ms_PoolDataCCharacter+3376>) at src/game/server/entities/character.cpp:1471
#8  0x000056404eaaaec8 in CCharacter::Tick (this=0x56404ed82b70 <ms_PoolDataCCharacter+3376>) at src/game/server/entities/character.cpp:2040
#9  0x000056404ea784ff in CGameWorld::Tick (this=0x7f4c97b23178) at src/game/server/gameworld.cpp:236
#10 0x000056404ea845da in CGameContext::OnTick (this=0x7f4c97b20010) at src/game/server/gamecontext.cpp:1031
#11 0x000056404ea51104 in CServer::Run (this=0x7f4c92863010) at src/engine/server/server.cpp:2213
#12 0x000056404ea54189 in main (argc=1, argv=0x7ffe9d10dfb8) at src/engine/server/server.cpp:2862

bretonium commented 5 years ago

I have found out that 6 ids are added but never removed on player connection-quit. So if the pool is 16k ids, i can reconnect 2666 times and waste the pool. 30 players need to reconnect 88 times to waste the pool. If each game takes ~3 minutes, maps are changed every 5 rounds, the pool will get spent after 22 hours. But actually faster, because each player can easily generate ~100 ids.

bretonium commented 5 years ago

Looks like these ids are created and not cleaned up here: https://github.com/yavl/teeworlds-infclassR/blob/master/src/game/server/entities/character.cpp#L69

teoman002 commented 5 years ago

@duralakun Thank you for your tool. It was very helpful for writing the fix. Breton you are right, it was the disconnect of a player.

Closed.

teoman002 commented 5 years ago

Forgot to ask if someone has time to patch that into necropotame master.

yavl commented 5 years ago

Also make a pull request for this crash fix to necro's master https://github.com/yavl/teeworlds-infclassR/commit/8e675751f1a4301ea22904abecf0f0aea0d057a4

yavl / teeworlds-infclassR

Server Crash - SnapNewID() #109