Crash on exit with MineClone 2 in "Host Server" mode

[grorp]

Minetest 5.9.0-dev-89f3502b56 (Linux)
Using Irrlicht 1.9.0mt14
Using LuaJIT 2.1.1692716794
Running on Linux/6.6.6-200.fc39.x86_64 x86_64
BUILD_TYPE=RelWithDebInfo
RUN_IN_PLACE=1
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
STATIC_SHAREDIR="."
STATIC_LOCALEDIR="locale"

Steps to reproduce

1. Start a local MineClone 2 game with "Host Server" enabled
2. Press "Exit to Menu" or "Exit to OS" in the pause menu

Result

ERROR[Server]: In thread 7fb8669c86c0:
ERROR[Server]: /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  0x000000000047fdfb in fatal_error_fn (msg=msg@entry=0xbf9d08 "Server::Send() missing peer ID", 
    file=<optimized out>, file@entry=0xbf9cd0 "/mt/src/server.cpp", 
    line=line@entry=1327, function=function@entry=0xbf9ca8 "void Server::Send(NetworkPacket*)")
    at /mt/src/debug.cpp:76
#4  0x0000000000a5feed in Server::Send (pkt=<optimized out>, this=<optimized out>)
    at /mt/src/server.cpp:1327
#5  0x0000000000a6399e in Server::Send (this=<optimized out>, pkt=<optimized out>)
    at /mt/src/server.cpp:1326
#6  0x0000000000a70cee in Server::SendInventory (this=0x1a58410, sao=<optimized out>, incremental=<optimized out>, 
    incremental@entry=true) at /mt/src/server.cpp:1483
#7  0x0000000000a99d41 in ServerEnvironment::step (this=<optimized out>, dtime=<optimized out>, 
    dtime@entry=0.0637599975) at /mt/src/serverenvironment.cpp:1648
#8  0x0000000000a7d3a6 in Server::AsyncRunStep (this=0x1a58410, dtime=dtime@entry=0.0637599975, 
    initial_step=initial_step@entry=false) at /mt/src/server.cpp:654
#9  0x0000000000a7f9d8 in ServerThread::run (this=0x1963740) at /mt/src/server.cpp:131
#10 0x00000000008b3262 in Thread::threadProc (thr=0x1963740)
    at /mt/src/threading/thread.cpp:189
#11 0x00007ffff70e31e3 in execute_native_thread_routine () from /lib64/libstdc++.so.6
#12 0x00007ffff6eac897 in start_thread () from /lib64/libc.so.6
#13 0x00007ffff6f336fc in clone3 () from /lib64/libc.so.6

[SmallJoker]

• PlayerSAO::unlinkPlayerSessionAndSave()
  ^ sets peer_id = 0 on shutdown
  • called by ServerEnvironment::removeRemovedObjects()
  • .. which is called before m_server->SendInventory in ServerEnvironment::step

Hence sao->getPeerId() == 0 (in progress of disconnect) but player->getPeerID() != 0

[sfan5]

Proposed fix:

diff --git a/src/server/clientiface.cpp b/src/server/clientiface.cpp
index 22e3e42d0..17363c91d 100644
--- a/src/server/clientiface.cpp
+++ b/src/server/clientiface.cpp
@@ -133,7 +133,7 @@ void RemoteClient::GetNextBlocks (
 		return;
 
 	PlayerSAO *sao = player->getPlayerSAO();
-	if (!sao)
+	if (!sao || sao->isGone())
 		return;
 
 	// Won't send anything if already sending
diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
index e0cf99378..113f6cb5d 100644
--- a/src/serverenvironment.cpp
+++ b/src/serverenvironment.cpp
@@ -1417,6 +1417,8 @@ void ServerEnvironment::step(float dtime)
 
 			PlayerSAO *playersao = player->getPlayerSAO();
 			assert(playersao);
+			if (playersao->isGone())
+				continue;
 
 			players.push_back(playersao);
 		}
@@ -1644,8 +1646,10 @@ void ServerEnvironment::step(float dtime)
 			continue;
 
 		PlayerSAO *sao = player->getPlayerSAO();
-		if (sao && player->inventory.checkModified())
-			m_server->SendInventory(sao, true);
+		if (sao && !sao->isGone()) {
+			if (player->inventory.checkModified())
+				m_server->SendInventory(sao, true);
+		}
 	}
 
 	// Send outdated detached inventories

There might be more places that need this check.

[sfan5]

How Server::SendInventory takes a PlayerSAO is fishy anyway because all required information is in RemotePlayer already.

[lhofhansl]

FWIW and unrelated. MC2 hangs upon shutdown in singleplayer mode too. That is new. If I get some time tomorrow I'll track it down.

Update: Stuck here:

#0  0x00007ffff6fca219 in __futex_abstimed_wait_common () from /lib64/libc.so.6
#1  0x00007ffff6fcf623 in __pthread_clockjoin_ex () from /lib64/libc.so.6
#2  0x00007ffff72e3227 in std::thread::join() () from /lib64/libstdc++.so.6
#3  0x00000000008c7534 in Thread::wait() ()
#4  0x00000000009a33a9 in EmergeManager::stopThreads() ()
#5  0x0000000000a9112f in Server::~Server() ()
#6  0x0000000000a91e19 in Server::~Server() ()
#7  0x0000000000534986 in Game::~Game() ()
#8  0x0000000000549e88 in the_game(bool*, InputHandler*, RenderingEngine*, GameStartData const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, ChatBackend&, bool*) ()
#9  0x00000000004e4d07 in ClientLauncher::run(GameStartData&, Settings const&) ()
#10 0x00000000004b565e in main ()

I have multiple emerge threads.