RemotePlayer: make peer ID always reflect the validity of PlayerSAO

[SmallJoker]

Upon disconnect, RemotePlayer still had a peer ID assigned even though the PlayerSAO object was maked as gone (for removal). This commit makes that the following always holds true:

(!sao || sao->isGone()) === (peer_id == PEER_ID_INEXISTENT)

Fixes #14313

To do

This PR is Ready for Review.

How to test

See #14313.

Note: Create a new MCL2 world to reproduce the bug with master. It did not trigger in a community-provided read-only world.

[sfan5]

another suggestion for a related clean up:

diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
--- a/src/server/player_sao.cpp
+++ b/src/server/player_sao.cpp
@@ -96,11 +96,12 @@ void PlayerSAO::addedToEnvironment(u32 dtime_s)
 void PlayerSAO::removingFromEnvironment()
 {
        ServerActiveObject::removingFromEnvironment();
-       if (m_player->getPlayerSAO() == this) {
-               unlinkPlayerSessionAndSave();
-               for (u32 attached_particle_spawner : m_attached_particle_spawners) {
-                       m_env->deleteParticleSpawner(attached_particle_spawner, false);
-               }
+       if (!m_player->getPlayerSAO())
+               return;
+       assert(m_player->getPlayerSAO() == this);
+       unlinkPlayerSessionAndSave();
+       for (u32 attached_particle_spawner : m_attached_particle_spawners) {
+               m_env->deleteParticleSpawner(attached_particle_spawner, false);
        }
 }
 

[sfan5]

looks good, not tested

[grorp]

I can still reproduce the crash with this PR, although it has become much rarer. This time, I tested with Mineclonia.

ERROR[Main]: In thread 7ffff5d76d00:
ERROR[Main]: /mt/src/server.cpp:52f: void Server::Send(NetworkPacket*): A fatal error occurred: Server::Send() missing peer ID

#0  0x00007ffff6eae834 in __pthread_kill_implementation () from /lib64/libc.so.6
#1  0x00007ffff6e5c8ee in raise () from /lib64/libc.so.6
#2  0x00007ffff6e448ff in abort () from /lib64/libc.so.6
#3  0x000000000047fe81 in fatal_error_fn (msg=msg@entry=0xbfade8 "Server::Send() missing peer ID", 
    file=<optimized out>, file@entry=0xbfadb0 "/mt/src/server.cpp", 
    line=line@entry=1327, function=function@entry=0xbfad88 "void Server::Send(NetworkPacket*)")
    at /mt/src/debug.cpp:76
#4  0x0000000000a5fe4d in Server::Send (pkt=0x7fffffffc7d0, this=0x1b5d3c0)
    at /mt/src/server.cpp:1327
#5  0x0000000000a6bfd7 in Server::Send (pkt=0x7fffffffc7d0, this=0x1b5d3c0)
    at /mt/src/server.cpp:1395
#6  Server::SendAccessDenied (reconnect=false, custom_reason="Server shutting down.", reason=<optimized out>, 
    peer_id=<optimized out>, this=0x1b5d3c0) at /mt/src/server.cpp:1399
#7  Server::DenyAccess (this=0x1b5d3c0, peer_id=<optimized out>, reason=<optimized out>, 
    custom_reason="Server shutting down.", reconnect=<optimized out>)
    at /mt/src/server.cpp:2868
#8  0x0000000000a8919f in ServerEnvironment::kickAllPlayers (this=0xbce5390, 
    reason=reason@entry=SERVER_ACCESSDENIED_SHUTDOWN, str_reason="Server shutting down.", 
    reconnect=reconnect@entry=false) at /mt/src/serverenvironment.cpp:632
#9  0x0000000000a7b4d2 in Server::~Server (this=0x1b5d3c0, __in_chrg=<optimized out>)
    at /mt/src/server.cpp:346
#10 0x0000000000a7c229 in Server::~Server (this=0x1b5d3c0, __in_chrg=<optimized out>)
    at /mt/src/server.cpp:404
#11 0x0000000000530376 in Game::~Game (this=0x7fffffffcac0, __in_chrg=<optimized out>)
    at /mt/src/client/game.cpp:1095
#12 0x0000000000545878 in the_game (kill=kill@entry=0xd3fe00 <porting::g_killed>, input=<optimized out>, 
    rendering_engine=<optimized out>, start_data=..., error_message="", chat_backend=..., 
    reconnect_requested=0x7fffffffce3f) at /mt/src/client/game.cpp:4626
#13 0x00000000004e06f7 in ClientLauncher::run (this=this@entry=0x7fffffffd5a0, start_data=..., cmd_args=...)
    at /mt/src/client/clientlauncher.cpp:256
#14 0x00000000004b0b38 in main (argc=<optimized out>, argv=<optimized out>)
    at /mt/src/client/clientlauncher.h:31

[SmallJoker]

@grorp I cannot reproduce this bug on my end, so please feel free to check whether the most recent commit works as expected.

[grorp]

Tested again, found no crashes.