diff --git a/justfile b/justfile index 4e923758..45c94c4b 100644 --- a/justfile +++ b/justfile @@ -279,7 +279,7 @@ test-auth-unit: unset VCPKG_ROOT cmake --preset {{ preset }} -DBUILD_TESTING=ON > /dev/null 2>&1 \ || cmake --preset {{ preset }} -DBUILD_TESTING=ON - cmake --build --preset {{ preset }} --target test_xpid_patch --target test_parse_endpoint --target test_behavioral --target test_token_auth --target test_messages --target test_crash_recovery --target test_nevr_config --target test_service_map --target test_social_facade --target test_scenario_early_quit --target test_early_quit_lockout --target test_schannel_cred_guard --target test_hooking --target test_plugin_load_plan --target test_system_module_loader --target test_login_redirect_override --target test_websocket_frame --target test_protobuf_transport --target test_url_diagnostics --target test_winhttp_stub --target test_callback_unregistration --target test_session_unregister --target test_mic_lifecycle --target test_telemetry_snapshot_store --target test_coop_ai_trace + cmake --build --preset {{ preset }} --target test_xpid_patch --target test_parse_endpoint --target test_behavioral --target test_token_auth --target test_messages --target test_crash_recovery --target test_nevr_config --target test_service_map --target test_social_facade --target test_scenario_early_quit --target test_early_quit_lockout --target test_schannel_cred_guard --target test_hooking --target test_plugin_load_plan --target test_system_module_loader --target test_login_redirect_override --target test_websocket_frame --target test_protobuf_transport --target test_url_diagnostics --target test_winhttp_stub --target test_callback_unregistration --target test_server_context --target test_session_unregister --target test_mic_lifecycle --target test_telemetry_snapshot_store --target test_coop_ai_trace cmake --build --preset {{ preset }} --target test_mic_dsp cmake --build --preset {{ preset }} --target test_game_image_guard bin="build/{{ preset }}/bin/test_xpid_patch.exe" @@ -384,7 +384,7 @@ test-auth-unit: exit 1 fi wine "$bin" - for test_name in test_system_module_loader test_login_redirect_override test_websocket_frame test_protobuf_transport test_url_diagnostics test_winhttp_stub test_callback_unregistration test_session_unregister test_mic_lifecycle test_telemetry_snapshot_store test_coop_ai_trace; do + for test_name in test_system_module_loader test_login_redirect_override test_websocket_frame test_protobuf_transport test_url_diagnostics test_winhttp_stub test_callback_unregistration test_server_context test_session_unregister test_mic_lifecycle test_telemetry_snapshot_store test_coop_ai_trace; do bin="build/{{ preset }}/bin/${test_name}.exe" if [[ ! -f "$bin" ]]; then echo "ERROR: GTest binary not found: $bin" >&2 diff --git a/src/abi/echovr.h b/src/abi/echovr.h index dc89de42..8ba9fefd 100644 --- a/src/abi/echovr.h +++ b/src/abi/echovr.h @@ -371,8 +371,11 @@ enum class NetGameState : INT32 { /// Lobby objects can be local, dedicated, etc. As a game server, this is a dedicated lobby object. /// struct Lobby { - /// Per-player data in the Lobby. Validated against echovr-reconstruction CServerConfig.h. - /// sizeof == 0xA0 (160 bytes) + /// Per-player data in the Lobby. Field offsets 0x00-0x9F match echovr-reconstruction + /// CServerConfig.h. sizeof == 0xD8 (216 bytes): echovr.exe indexes the [lobby+0x360] + /// array with a 0xD8 stride — CNSLobby::SmiteEntrant 0x14061665d `IMUL RAX,RDX,0xd8`, + /// fcn_1406082b0 0x1406082c9 `IMUL RDX,RDX,0xd8`, 0x140616920 loop 0x14061698f + /// `ADD R8,0xd8`. The reconstruction's 0xA0 is the mapped prefix, not the element size. struct EntrantData { XPlatformId userId; // +0x00 SymbolId platformId; // +0x10 @@ -388,6 +391,7 @@ struct Lobby { UINT16 genIndex; // +0x8C UINT16 teamIndex; // +0x8E (0=blue, 1=orange, 2=spec) Json json; // +0x90 (root + cache pointers, 0x10 bytes) + BYTE _unkA0[0x38]; // +0xA0 unmapped tail of the 0xD8-byte element }; /// Validated against echovr-reconstruction CServerConfig.h. sizeof == 0x38 @@ -452,7 +456,7 @@ struct Lobby { }; // --- Lobby sub-struct validation (echovr-reconstruction CServerConfig.h, CLobby.h) --- -static_assert(sizeof(Lobby::EntrantData) == 0xA0, "EntrantData size mismatch with reconstruction"); +static_assert(sizeof(Lobby::EntrantData) == 0xD8, "EntrantData stride mismatch with echovr.exe (0x14061665d)"); static_assert(offsetof(Lobby::EntrantData, userId) == 0x00, "EntrantData::userId offset mismatch"); static_assert(offsetof(Lobby::EntrantData, platformId) == 0x10, "EntrantData::platformId offset mismatch"); static_assert(offsetof(Lobby::EntrantData, uniqueName) == 0x18, "EntrantData::uniqueName offset mismatch"); diff --git a/src/runtime/CMakeLists.txt b/src/runtime/CMakeLists.txt index 8ac593a7..77fbf67a 100644 --- a/src/runtime/CMakeLists.txt +++ b/src/runtime/CMakeLists.txt @@ -338,6 +338,16 @@ if(BUILD_TESTING) target_link_libraries(test_callback_unregistration PRIVATE GTest::gtest GTest::gtest_main) gtest_discover_tests(test_callback_unregistration DISCOVERY_MODE PRE_TEST) + # Issue #38: entrant accessors read the lobby's live entrant array. + add_executable(test_server_context + tests/test_server_context.cpp + server/server_context.cpp) + set_source_files_properties(tests/test_server_context.cpp + PROPERTIES SKIP_PRECOMPILE_HEADERS ON) + target_include_directories(test_server_context PRIVATE ${CMAKE_SOURCE_DIR}/src) + target_link_libraries(test_server_context PRIVATE GTest::gtest GTest::gtest_main) + gtest_discover_tests(test_server_context DISCOVERY_MODE PRE_TEST) + add_executable(test_session_unregister tests/test_session_unregister.cpp server/session_unregister.cpp diff --git a/src/runtime/server/gameserver.cpp b/src/runtime/server/gameserver.cpp index 67cedb43..b927841a 100644 --- a/src/runtime/server/gameserver.cpp +++ b/src/runtime/server/gameserver.cpp @@ -418,11 +418,15 @@ void OnTcpMsgProtobuf(GameServerLib* self, VOID*, EchoVR::TcpPeer, const VOID* m } if (!found) { - Log(EchoVR::LogLevel::Warning, "[NEVR.GAMESERVER] Smite entrant not found in lobby: %s", - smite.entrant_id().c_str()); + Log(EchoVR::LogLevel::Warning, "[NEVR.GAMESERVER] Smite entrant not found in lobby: %s entrants=%llu", + smite.entrant_id().c_str(), static_cast(entrantCount)); break; } + Log(EchoVR::LogLevel::Info, "[NEVR.GAMESERVER] Smite entrant resolved: entrant=%s slot=%llu entrants=%llu", + smite.entrant_id().c_str(), static_cast(slotIndex), + static_cast(entrantCount)); + if (broadcaster) { auto encoded = EncodeLobbySmiteEntrant(slotIndex); EchoVR::BroadcasterReceiveLocalEvent(broadcaster, Sym::LobbySmiteEntrant, "SNSLobbySmiteEntrant", diff --git a/src/runtime/server/server_context.cpp b/src/runtime/server/server_context.cpp index 6b3d9740..5d920a05 100644 --- a/src/runtime/server/server_context.cpp +++ b/src/runtime/server/server_context.cpp @@ -43,14 +43,6 @@ void SessionState::Reset() { void ServerContext::Initialize(EchoVR::Lobby* lobby, EchoVR::Broadcaster* broadcaster) { std::unique_lock lock(m_stateMutex); - if (lobby && lobby->entrantData.items) { - m_cachedEntrants.clear(); - m_cachedEntrants.reserve(lobby->entrantData.count); - for (uint64_t i = 0; i < lobby->entrantData.count; ++i) { - m_cachedEntrants.push_back(lobby->entrantData.items[i]); - } - } - m_lobby = lobby; m_broadcaster = broadcaster; @@ -75,7 +67,6 @@ void ServerContext::Terminate() { m_state = ServerState::Terminated; m_lobby = nullptr; m_broadcaster = nullptr; - m_cachedEntrants.clear(); m_serverDbPeer = EchoVR::TcpPeer_InvalidPeer; { @@ -179,28 +170,39 @@ EchoVR::TcpBroadcasterData* ServerContext::GetTcpBroadcaster() const { return nullptr; } +// Entrants are read from the lobby's live array on every call (issue #38). The +// game hands us the lobby in IServerLib::Initialize, from CNSLobby +// LoadServerSupport (0x14060bb70) during boot, before anyone has joined, so a +// copy taken there stays empty. CNSLobby::Update (0x140617890) calls +// IServerLib::Update first (0x1406178c3) and then mutates [lobby+0x360] itself, +// on the same thread, so a read made inside one of our callbacks sees a +// consistent array. EchoVR::Lobby::EntrantData* ServerContext::GetEntrant(uint32_t index) const { std::shared_lock lock(m_stateMutex); - if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated) { + if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated || !m_lobby) { return nullptr; } - if (index >= m_cachedEntrants.size()) { + const auto& entrants = m_lobby->entrantData; + if (!entrants.items || index >= entrants.count) { return nullptr; } - return const_cast(&m_cachedEntrants[index]); + return &entrants.items[index]; } uint64_t ServerContext::GetEntrantCount() const { std::shared_lock lock(m_stateMutex); - if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated) { + if (m_state == ServerState::Uninitialized || m_state == ServerState::Terminated || !m_lobby) { return 0; } - return m_cachedEntrants.size(); + // Never report a count we could not index (echovr-reconstruction + // CNSLobby.cpp:473 frees the array and zeroes items and count together). + const auto& entrants = m_lobby->entrantData; + return entrants.items ? entrants.count : 0; } void ServerContext::SetServerDbPeer(const EchoVR::TcpPeer& peer) { diff --git a/src/runtime/server/server_context.h b/src/runtime/server/server_context.h index ef2b377e..671d3737 100644 --- a/src/runtime/server/server_context.h +++ b/src/runtime/server/server_context.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "abi/echovr.h" #include "core/pch.h" @@ -100,8 +99,11 @@ class ServerContext { EchoVR::Broadcaster* GetBroadcaster() const; EchoVR::TcpBroadcasterData* GetTcpBroadcaster() const; - // Safe entrant access with bounds checking (shared lock) - // Returns nullptr if index out of bounds or not initialized + // Entrant access, read live from the lobby's entrant array on every call + // (issue #38). Returns nullptr if index is out of bounds or not initialized. + // The pointer is into game memory: use it within the current game-thread + // callback and never store it — the game may free or rewrite the array on + // its next tick. EchoVR::Lobby::EntrantData* GetEntrant(uint32_t index) const; uint64_t GetEntrantCount() const; @@ -130,9 +132,6 @@ class ServerContext { EchoVR::Lobby* m_lobby = nullptr; EchoVR::Broadcaster* m_broadcaster = nullptr; - // Cached entrant data (owns the data, not pointers from game) - std::vector m_cachedEntrants; - // ServerDB connection EchoVR::TcpPeer m_serverDbPeer = EchoVR::TcpPeer_InvalidPeer; diff --git a/src/runtime/tests/test_server_context.cpp b/src/runtime/tests/test_server_context.cpp new file mode 100644 index 00000000..5d8186fb --- /dev/null +++ b/src/runtime/tests/test_server_context.cpp @@ -0,0 +1,145 @@ +// ServerContext entrant access (issue #38). +// +// The entrant accessors must answer from the lobby's live entrant array, not +// from a copy taken at IServerLib::Initialize. The game calls Initialize from +// CNSLobby LoadServerSupport (echovr.exe 0x14060bb70) while the server is still +// booting, before any player has joined, so a copy taken there is empty for +// the life of the process. Every test below mutates the fake lobby AFTER +// Initialize and asserts the accessors see the mutation. +#include "runtime/server/server_context.h" + +#include + +#include +#include +#include + +namespace { + +using Entrant = EchoVR::Lobby::EntrantData; + +// Points the fake lobby's entrant HeapArray at `entrants`. +void Attach(EchoVR::Lobby& lobby, std::vector& entrants) { + lobby.entrantData.items = entrants.empty() ? nullptr : entrants.data(); + lobby.entrantData.count = entrants.size(); +} + +Entrant MakeEntrant(uint64_t accountId) { + Entrant entrant{}; + entrant.userId.accountId = accountId; + return entrant; +} + +} // namespace + +TEST(ServerContextEntrants, JoinAfterInitializeIsVisible) { + EchoVR::Lobby lobby{}; + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + ASSERT_EQ(context.GetEntrantCount(), 0U); + + std::vector entrants = {MakeEntrant(11), MakeEntrant(22), MakeEntrant(33)}; + Attach(lobby, entrants); + + EXPECT_EQ(context.GetEntrantCount(), 3U); + ASSERT_NE(context.GetEntrant(2), nullptr); + EXPECT_EQ(context.GetEntrant(2)->userId.accountId, 33U); +} + +TEST(ServerContextEntrants, LeaveAfterInitializeIsVisible) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11), MakeEntrant(22), MakeEntrant(33)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + ASSERT_EQ(context.GetEntrantCount(), 3U); + + lobby.entrantData.count = 1; + + EXPECT_EQ(context.GetEntrantCount(), 1U); + EXPECT_EQ(context.GetEntrant(1), nullptr); + EXPECT_EQ(context.GetEntrant(2), nullptr); +} + +TEST(ServerContextEntrants, SlotReusedByAnotherPlayerReportsTheNewPlayer) { + // The smite path resolves an entrant to a slot index and hands that index to + // the game. A stale copy here would name the slot's previous occupant. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + entrants[0].userId.accountId = 99; + + ASSERT_NE(context.GetEntrant(0), nullptr); + EXPECT_EQ(context.GetEntrant(0)->userId.accountId, 99U); +} + +TEST(ServerContextEntrants, ReturnsTheGameElementNotACopy) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11), MakeEntrant(22)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + EXPECT_EQ(context.GetEntrant(0), &entrants[0]); + EXPECT_EQ(context.GetEntrant(1), &entrants[1]); +} + +TEST(ServerContextEntrants, ElementStrideMatchesTheBinary) { + // echovr.exe indexes [lobby+0x360] with a 0xD8 stride: CNSLobby::SmiteEntrant + // 0x14061665d IMUL RAX,RDX,0xd8; fcn_1406082b0 0x1406082c9 IMUL RDX,RDX,0xd8; + // 0x14061698f ADD R8,0xd8. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(1), MakeEntrant(2)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + const auto* first = reinterpret_cast(context.GetEntrant(0)); + const auto* second = reinterpret_cast(context.GetEntrant(1)); + ASSERT_NE(first, nullptr); + ASSERT_NE(second, nullptr); + EXPECT_EQ(second - first, 0xD8); +} + +TEST(ServerContextEntrants, FreedArrayReadsAsEmpty) { + // The game frees the array and zeroes items/count together (CNSLobby.cpp:473 + // in echovr-reconstruction). A null items pointer must never be indexed even + // if count is stale. + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + + lobby.entrantData.items = nullptr; + + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); +} + +TEST(ServerContextEntrants, UninitializedAndTerminatedReadAsEmpty) { + EchoVR::Lobby lobby{}; + std::vector entrants = {MakeEntrant(11)}; + Attach(lobby, entrants); + GameServer::ServerContext context; + + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); + + context.Initialize(&lobby, nullptr); + context.FinalizeInitialization(); + EXPECT_EQ(context.GetEntrantCount(), 1U); + + context.Terminate(); + EXPECT_EQ(context.GetEntrantCount(), 0U); + EXPECT_EQ(context.GetEntrant(0), nullptr); +} diff --git a/tools/tests/test_release_contract.py b/tools/tests/test_release_contract.py index 8c145d6c..1c887b22 100644 --- a/tools/tests/test_release_contract.py +++ b/tools/tests/test_release_contract.py @@ -39,6 +39,7 @@ def test_new_runtime_gtests_are_built_and_run_by_auth_unit_gate(self): "test_url_diagnostics", "test_winhttp_stub", "test_callback_unregistration", + "test_server_context", "test_session_unregister", "test_mic_lifecycle", "test_telemetry_snapshot_store",