diff --git a/gamelift-server-sdk-tests/source/aws/gamelift/internal/network/GameLiftWebSocketClientManagerTest.cpp b/gamelift-server-sdk-tests/source/aws/gamelift/internal/network/GameLiftWebSocketClientManagerTest.cpp index bead86a..7da0bbc 100644 --- a/gamelift-server-sdk-tests/source/aws/gamelift/internal/network/GameLiftWebSocketClientManagerTest.cpp +++ b/gamelift-server-sdk-tests/source/aws/gamelift/internal/network/GameLiftWebSocketClientManagerTest.cpp @@ -19,7 +19,7 @@ namespace GameLift { namespace Internal { namespace Test { -static const std::string sdkVersion = "5.5.0"; +static const std::string sdkVersion = "5.6.0"; // private constants copied over for testing purposes static constexpr const char *PID_KEY = "pID"; static constexpr const char *SDK_VERSION_KEY = "sdkVersion"; diff --git a/gamelift-server-sdk-tests/source/aws/gamelift/internal/util/LoggerHelperTest.cpp b/gamelift-server-sdk-tests/source/aws/gamelift/internal/util/LoggerHelperTest.cpp new file mode 100644 index 0000000..1d6c90d --- /dev/null +++ b/gamelift-server-sdk-tests/source/aws/gamelift/internal/util/LoggerHelperTest.cpp @@ -0,0 +1,733 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef _WIN32 +#include +#include +#define rmdir _rmdir +#else +#include +#endif + +namespace { +// Cross-platform recursive directory removal (C++11 compatible) +void RemoveDirectoryRecursive(const std::string& path) { +#ifdef _WIN32 + std::system(("rmdir /s /q \"" + path + "\" 2>nul").c_str()); +#else + std::system(("rm -rf '" + path + "'").c_str()); +#endif +} + +// Cross-platform directory creation (C++11 compatible) +void EnsureDirectoryExists(const std::string& path) { +#ifdef _WIN32 + _mkdir(path.c_str()); +#else + std::system(("mkdir -p '" + path + "'").c_str()); +#endif +} +} // anonymous namespace + +namespace Aws { +namespace GameLift { +namespace Internal { +namespace Test { + +class LoggerHelperTest : public ::testing::Test { +protected: + void SetUp() override { + // Reset the process-static custom-logger flag so each test starts from a clean + // slate. Without this, the first test that registers a callback logger leaves + // s_customLoggerRegistered=true, causing every subsequent InitializeCallbackLogger + // call to fail its CAS with ALREADY_INITIALIZED. + LoggerHelper::ResetCustomLoggerRegistered(); + // Drop all loggers and restore a fresh default to ensure clean state + spdlog::drop_all(); + spdlog::set_default_logger(spdlog::stdout_color_mt("default_test")); + } + + void TearDown() override { + // Clean up test log files + std::remove("logs/gamelift-server-sdk-test-logger.log"); + // Reset the custom-logger flag so it does not leak into the next test. + LoggerHelper::ResetCustomLoggerRegistered(); + // Restore a valid default logger so subsequent tests that use spdlog don't segfault + spdlog::drop_all(); + spdlog::set_default_logger(spdlog::stdout_color_mt("default_test")); + } +}; + +TEST_F(LoggerHelperTest, GIVEN_validLogDirectory_WHEN_initializeLogger_THEN_returnsSuccess) { + // GIVEN - ensure logs/ is writable (remove any leftover from previous tests) + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-logger"); + + // THEN + EXPECT_TRUE(outcome.IsSuccess()); + EXPECT_NE(spdlog::get("multi_sink"), nullptr); +} + +TEST_F(LoggerHelperTest, GIVEN_validLogDirectory_WHEN_initializeLogger_THEN_logFileIsCreated) { + // GIVEN - ensure logs/ is writable + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + + // WHEN + LoggerHelper::InitializeLogger("test-logger"); + spdlog::default_logger()->flush(); + + // THEN + std::ifstream logFile("logs/gamelift-server-sdk-test-logger.log"); + EXPECT_TRUE(logFile.good()); +} + +TEST_F(LoggerHelperTest, GIVEN_invalidLogPath_WHEN_initializeLogger_THEN_returnsError) { + // GIVEN - create a path that will fail file creation +#ifdef _WIN32 + RemoveDirectoryA("logs"); + DeleteFileA("logs"); + // Create the logs directory and place a read-only file at the exact path spdlog will try to open + CreateDirectoryA("logs", NULL); + HANDLE hFile = CreateFileA("logs\\gamelift-server-sdk-test-logger.log", + GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_READONLY, NULL); + if (hFile != INVALID_HANDLE_VALUE) CloseHandle(hFile); +#else + // Remove logs/ directory and all contents first + RemoveDirectoryRecursive("logs"); + symlink("/proc/nonexistent/deeply/nested/invalid/path", "logs"); +#endif + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-logger"); + + // THEN + EXPECT_FALSE(outcome.IsSuccess()); + std::string errorMsg = outcome.GetError().GetErrorMessage(); + EXPECT_NE(errorMsg.find("Failed to initialize logger"), std::string::npos); + + // Cleanup +#ifdef _WIN32 + SetFileAttributesA("logs\\gamelift-server-sdk-test-logger.log", FILE_ATTRIBUTE_NORMAL); + DeleteFileA("logs\\gamelift-server-sdk-test-logger.log"); + RemoveDirectoryA("logs"); +#else + unlink("logs"); +#endif +} + +TEST_F(LoggerHelperTest, GIVEN_loggerAlreadyInitialized_WHEN_initializeLoggerCalledAgain_THEN_reusesExistingLogger) { + // GIVEN - clean slate, then initialize logger once + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + LoggerHelper::InitializeLogger("first-call"); + auto firstLogger = spdlog::get("multi_sink"); + ASSERT_NE(firstLogger, nullptr); + + // WHEN - initialize again with a different process ID + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("second-call"); + + // THEN - should succeed but reuse the existing logger (no duplicate) + EXPECT_TRUE(outcome.IsSuccess()); + auto secondLogger = spdlog::get("multi_sink"); + EXPECT_EQ(firstLogger.get(), secondLogger.get()); + // Second log file should NOT be created + EXPECT_FALSE(std::ifstream("logs/gamelift-server-sdk-second-call.log").good()); +} + +// Thread-safety note: This helper is safe for single-threaded test scenarios only. +// The CallbackSink (base_sink) serializes callback invocations, so no +// additional synchronization is needed for tests using the default spdlog sink. +// For multi-threaded tests, use a mutex-guarded callback instead (see the +// GIVEN_customCallback_WHEN_multipleThreadsLog test below). +struct TestLogCapture { + std::vector> messages; + + static void Callback(Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + auto* capture = static_cast(userData); + capture->messages.emplace_back(level, std::string(message)); + } +}; + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_initializeLogger_THEN_callbackReceivesLogMessages) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-callback", params); + ASSERT_TRUE(outcome.IsSuccess()); + spdlog::info("hello from test"); + spdlog::default_logger()->flush(); + + // THEN + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Info); + EXPECT_NE(capture.messages.back().second.find("hello from test"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_initializeLogger_THEN_noLogFileCreated) { + // GIVEN - ensure logs/ is clean + RemoveDirectoryRecursive("logs"); + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-no-file", params); + ASSERT_TRUE(outcome.IsSuccess()); + spdlog::info("should not go to file"); + spdlog::default_logger()->flush(); + + // THEN - no log file should be created + EXPECT_FALSE(std::ifstream("logs/gamelift-server-sdk-test-no-file.log").good()); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithMinLevelWarn_WHEN_infoIsLogged_THEN_callbackNotCalled) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Warn); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-level-filter", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::info("this info message should be filtered out"); + spdlog::default_logger()->flush(); + + // THEN + EXPECT_TRUE(capture.messages.empty()); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithMinLevelWarn_WHEN_warnIsLogged_THEN_callbackCalled) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Warn); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-warn-level", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::warn("this warn should arrive"); + spdlog::default_logger()->flush(); + + // THEN + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Warn); + EXPECT_NE(capture.messages.back().second.find("this warn should arrive"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithMinLevelDebug_WHEN_debugIsLogged_THEN_callbackCalled) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Debug); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-debug-level", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::debug("debug message for testing"); + spdlog::default_logger()->flush(); + + // THEN + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Debug); + EXPECT_NE(capture.messages.back().second.find("debug message for testing"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_errorIsLogged_THEN_callbackReceivesErrorLevel) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Debug); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-error-level", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::error("error message"); + spdlog::default_logger()->flush(); + + // THEN + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Error); + EXPECT_NE(capture.messages.back().second.find("error message"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_criticalIsLogged_THEN_callbackReceivesFatalLevel) { + // GIVEN + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Debug); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-critical-level", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::critical("critical failure"); + spdlog::default_logger()->flush(); + + // THEN - critical maps to Fatal in the CallbackSink + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Fatal); + EXPECT_NE(capture.messages.back().second.find("critical failure"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithUserData_WHEN_logIsCalled_THEN_userDataIsPassedThrough) { + // GIVEN + struct UserContext { + int id; + bool callbackInvoked; + }; + UserContext ctx{42, false}; + + auto verifyCallback = [](Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + auto* uctx = static_cast(userData); + uctx->callbackInvoked = true; + }; + + Aws::GameLift::Server::CustomLoggerConfiguration params(verifyCallback, &ctx, Aws::GameLift::Server::LogLevel::Info); + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-userdata", params); + ASSERT_TRUE(outcome.IsSuccess()); + spdlog::info("trigger callback"); + spdlog::default_logger()->flush(); + + // THEN + EXPECT_TRUE(ctx.callbackInvoked); + EXPECT_EQ(ctx.id, 42); +} + +TEST_F(LoggerHelperTest, GIVEN_nullCallback_WHEN_initializeLogger_THEN_defaultBehavior) { + // This test validates the internal LoggerHelper's graceful fallback behavior: + // when a null callback is passed directly to the helper (bypassing Server::InitCustomLogger), + // it falls through to the default file/stdout logger rather than crashing. + // Note: The public API (Server::InitCustomLogger) rejects null callbacks with BAD_REQUEST; + // see GameLiftServerAPITest for that coverage. + + // GIVEN - ensure logs/ is clean + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + Aws::GameLift::Server::CustomLoggerConfiguration params(nullptr, nullptr, Aws::GameLift::Server::LogLevel::Info); + + // WHEN + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-null-cb", params); + ASSERT_TRUE(outcome.IsSuccess()); + spdlog::default_logger()->flush(); + + // THEN - should fall back to default behavior (file is created) + std::ifstream logFile("logs/gamelift-server-sdk-test-null-cb.log"); + EXPECT_TRUE(logFile.good()); +} + +// NOTE: This exercises the internal helper's ability to replace loggers directly. +// The public Server::InitCustomLogger() API prevents re-initialization via an atomic CAS +// guard and would return ALREADY_INITIALIZED in this scenario. +TEST_F(LoggerHelperTest, GIVEN_loggerAlreadyInitialized_WHEN_initializeWithCallback_THEN_callbackReplacesExistingLogger) { + // GIVEN - initialize with default behavior first + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + LoggerHelper::InitializeLogger("test-first-init"); + ASSERT_NE(spdlog::get("multi_sink"), nullptr); + + // WHEN - initialize again with a callback + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-second-init", params); + + // THEN - callback logger replaces the existing one + EXPECT_TRUE(outcome.IsSuccess()); + spdlog::info("this should now go to the callback"); + spdlog::default_logger()->flush(); + EXPECT_FALSE(capture.messages.empty()); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_multipleThreadsLog_THEN_noDataRaceOrCrash) { + // GIVEN + std::mutex captureMutex; + std::vector capturedMessages; + + auto threadSafeCallback = [](Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + auto* data = static_cast*>*>(userData); + std::lock_guard lock(*data->first); + data->second->push_back(std::string(message)); + }; + + std::pair*> callbackData{&captureMutex, &capturedMessages}; + Aws::GameLift::Server::CustomLoggerConfiguration params(threadSafeCallback, &callbackData, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-threadsafe", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN - log from multiple threads concurrently + const int numThreads = 4; + const int messagesPerThread = 50; + std::vector threads; + for (int t = 0; t < numThreads; ++t) { + threads.emplace_back([t, messagesPerThread]() { + for (int i = 0; i < messagesPerThread; ++i) { + spdlog::info("thread {} message {}", t, i); + } + }); + } + for (auto& th : threads) { + th.join(); + } + spdlog::default_logger()->flush(); + + // THEN - all messages should arrive (no data race, no crash) + std::lock_guard lock(captureMutex); + EXPECT_EQ(static_cast(capturedMessages.size()), numThreads * messagesPerThread); +} + +TEST_F(LoggerHelperTest, GIVEN_throwingCallback_WHEN_logIsCalled_THEN_sdkDoesNotCrash) { + // GIVEN + auto throwingCallback = [](Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + throw std::runtime_error("callback failure"); + }; + + Aws::GameLift::Server::CustomLoggerConfiguration params(throwingCallback, nullptr, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-throwing", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN/THEN - logging should not crash the process + EXPECT_NO_THROW({ + spdlog::info("this should not crash"); + spdlog::default_logger()->flush(); + }); + + // Verify we can still log after the exception + EXPECT_NO_THROW({ + spdlog::info("second message after exception"); + spdlog::default_logger()->flush(); + }); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_dropAllCalledThenLogAttempted_THEN_noCrashAndCallbackNotInvoked) { + // GIVEN - Initialize logger with a callback, simulating the InitSDK → Destroy lifecycle. + // INVARIANT: After the SDK logger is dropped, any subsequent spdlog call must route to + // the stdout fallback -- no crash, no callback invocation, no use-after-free. + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-teardown", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // Verify callback works before teardown + spdlog::info("before teardown"); + spdlog::default_logger()->flush(); + ASSERT_FALSE(capture.messages.empty()); + size_t messageCountBeforeDrop = capture.messages.size(); + + // WHEN - Tear down the logger using the hardened sequence (as DestroyInstance() does): + // Install a fallback BEFORE dropping the SDK logger so spdlog's default is never null. + auto fallback = std::make_shared( + "fallback", std::make_shared()); + spdlog::set_default_logger(fallback); + LoggerHelper::DropSdkLogger(); + + // THEN - Logging after teardown should NOT crash and should NOT invoke the callback. + EXPECT_NO_THROW({ + spdlog::info("after drop - should not crash"); + spdlog::warn("another log after drop"); + }); + + // The callback must NOT have been invoked after drop + EXPECT_EQ(capture.messages.size(), messageCountBeforeDrop); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_dropAllCalledFromAnotherThread_THEN_noCrashOrUseAfterFree) { + // GIVEN - This test exercises the teardown-safety mechanism in isolation by joining its + // own helper thread before dropping. NOTE: production Destroy() does NOT join the detached + // game-session/terminate/update handler threads, so this test validates the fallback-then-drop + // sequence only for the already-quiesced case -- it does not prove the detached-thread race is + // eliminated (see CustomLoggerConfiguration.h and the DestroyInstance comment). + std::atomic stopLogging{false}; + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-race-teardown", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN - Start a background thread that logs continuously + std::thread loggerThread([&stopLogging]() { + while (!stopLogging.load(std::memory_order_acquire)) { + spdlog::info("background thread logging"); + std::this_thread::sleep_for(std::chrono::microseconds(100)); + } + }); + + // Let the logger thread run for a bit + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + + // Signal the thread to stop (simulating thread join before teardown, as in the fixed code) + stopLogging.store(true, std::memory_order_release); + loggerThread.join(); + + // THEN - After thread is joined, hardened teardown is safe: fallback first, then drop. + auto fallback = std::make_shared( + "fallback", std::make_shared()); + EXPECT_NO_THROW({ spdlog::set_default_logger(fallback); }); + EXPECT_NO_THROW({ spdlog::drop("multi_sink"); }); + + // No more callbacks should fire + size_t finalCount = capture.messages.size(); + EXPECT_GT(finalCount, 0u); // Should have captured messages from the background thread + + // Logging after teardown must not crash and must not invoke the callback + EXPECT_NO_THROW({ spdlog::info("post-drop logging"); }); + EXPECT_EQ(capture.messages.size(), finalCount); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallback_WHEN_messageIsLogged_THEN_formatContainsOnlyMessageBody) { + // GIVEN - The callback sink uses pattern "%v" (message body only). + // This test locks down the contract: no timestamps, no level prefix, no thread id. + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-format-contract", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::info("exact message content"); + spdlog::default_logger()->flush(); + + // THEN - message should be exactly the body text, no timestamp or level prefix. + // The callback sink uses "%v" format (message-only), so the output must match verbatim. + ASSERT_FALSE(capture.messages.empty()); + const std::string& msg = capture.messages.back().second; + EXPECT_EQ(msg, "exact message content"); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithMinLevelOff_WHEN_allLevelsLogged_THEN_callbackNeverInvoked) { + // GIVEN - LogLevel::Off disables all callback dispatching + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Off); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-level-off", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN - log at every severity + spdlog::trace("trace msg"); + spdlog::debug("debug msg"); + spdlog::info("info msg"); + spdlog::warn("warn msg"); + spdlog::error("error msg"); + spdlog::critical("critical msg"); + spdlog::default_logger()->flush(); + + // THEN - callback must never have been invoked + EXPECT_TRUE(capture.messages.empty()); +} + +TEST_F(LoggerHelperTest, GIVEN_customCallbackWithMinLevelTrace_WHEN_traceIsLogged_THEN_callbackCalled) { + // GIVEN - LogLevel::Trace is the lowest level; everything passes through + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Trace); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-trace-level", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN + spdlog::trace("trace message for testing"); + spdlog::default_logger()->flush(); + + // THEN + ASSERT_FALSE(capture.messages.empty()); + EXPECT_EQ(capture.messages.back().first, Aws::GameLift::Server::LogLevel::Trace); + EXPECT_NE(capture.messages.back().second.find("trace message for testing"), std::string::npos); +} + +TEST_F(LoggerHelperTest, GIVEN_callbackThrowingNonStdException_WHEN_logIsCalled_THEN_sdkDoesNotCrash) { + // GIVEN - A callback that throws a non-std::exception type (e.g., int). + // This exercises the catch(...) handler in CallbackSink::sink_it_(). + auto throwingIntCallback = [](Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + throw 42; // non-std exception + }; + + Aws::GameLift::Server::CustomLoggerConfiguration params(throwingIntCallback, nullptr, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome outcome = LoggerHelper::InitializeLogger("test-throw-int", params); + ASSERT_TRUE(outcome.IsSuccess()); + + // WHEN/THEN - logging must not crash despite non-std throw + EXPECT_NO_THROW({ + spdlog::info("this triggers throw 42"); + spdlog::default_logger()->flush(); + }); + + // Verify continued logging works after the non-std exception + EXPECT_NO_THROW({ + spdlog::info("still alive after throw int"); + spdlog::default_logger()->flush(); + }); +} + +// ===== New tests for sink-level indirection design ===== + +TEST_F(LoggerHelperTest, GIVEN_defaultLoggerInitialized_WHEN_callbackLoggerRegistered_THEN_subsequentLogsHitCallbackNotFile) { + // GIVEN - Initialize default logger (stdout + file sinks via dist_sink_mt) + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + Aws::GameLift::GenericOutcome initOutcome = LoggerHelper::InitializeLogger("test-swap"); + ASSERT_TRUE(initOutcome.IsSuccess()); + + // Log one message that should go to the file + spdlog::info("message-before-swap"); + spdlog::default_logger()->flush(); + + // Verify the file was created and has content + { + std::ifstream logFile("logs/gamelift-server-sdk-test-swap.log"); + ASSERT_TRUE(logFile.good()); + std::string content((std::istreambuf_iterator(logFile)), + std::istreambuf_iterator()); + EXPECT_NE(content.find("message-before-swap"), std::string::npos); + } + + // WHEN - Register a callback logger, which should swap the dist_sink's children + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + Aws::GameLift::GenericOutcome callbackOutcome = LoggerHelper::InitializeCallbackLogger(params); + ASSERT_TRUE(callbackOutcome.IsSuccess()); + + // Log after the swap + spdlog::info("message-after-swap"); + spdlog::default_logger()->flush(); + + // THEN - The callback should have received the post-swap message + ASSERT_FALSE(capture.messages.empty()); + EXPECT_NE(capture.messages.back().second.find("message-after-swap"), std::string::npos); + + // The logger instance should be the SAME (no drop/replace, just sink swap) + EXPECT_NE(spdlog::get("multi_sink"), nullptr); + + // The default logger should still have a dist_sink_mt as its first sink + auto logger = spdlog::default_logger(); + ASSERT_FALSE(logger->sinks().empty()); + auto distSink = std::dynamic_pointer_cast(logger->sinks().front()); + EXPECT_NE(distSink, nullptr); +} + +TEST_F(LoggerHelperTest, GIVEN_defaultLoggerActive_WHEN_callbackRegisteredWhileThreadsLog_THEN_noCrashAndCallbackReceivesPostSwapMessages) { + // GIVEN - Concurrency smoke test: start N threads logging via the default logger, + // then mid-stream swap to a callback logger. Validates that the dist_sink_mt + // serialization prevents crashes during the swap. + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + Aws::GameLift::GenericOutcome initOutcome = LoggerHelper::InitializeLogger("test-concurrent-swap"); + ASSERT_TRUE(initOutcome.IsSuccess()); + + std::atomic stopLogging{false}; + std::atomic messagesLogged{0}; + const int numThreads = 4; + + // Start background threads that continuously log + std::vector threads; + for (int t = 0; t < numThreads; ++t) { + threads.emplace_back([&stopLogging, &messagesLogged, t]() { + while (!stopLogging.load(std::memory_order_acquire)) { + spdlog::info("concurrent thread {} message {}", t, messagesLogged.fetch_add(1, std::memory_order_relaxed)); + std::this_thread::sleep_for(std::chrono::microseconds(50)); + } + }); + } + + // Let threads log for a bit through the default (stdout+file) path + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + + // WHEN - Swap to callback logger mid-stream + std::mutex captureMutex; + std::vector capturedMessages; + auto threadSafeCallback = [](Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + auto* data = static_cast*>*>(userData); + std::lock_guard lock(*data->first); + data->second->push_back(std::string(message)); + }; + std::pair*> callbackData{&captureMutex, &capturedMessages}; + Aws::GameLift::Server::CustomLoggerConfiguration params(threadSafeCallback, &callbackData, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome callbackOutcome = LoggerHelper::InitializeCallbackLogger(params); + EXPECT_TRUE(callbackOutcome.IsSuccess()); + + // Let threads log through the callback path for a bit + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + + // Stop and join + stopLogging.store(true, std::memory_order_release); + for (auto& th : threads) { + th.join(); + } + + // THEN - No crash, and the callback received some messages after the swap + std::lock_guard lock(captureMutex); + EXPECT_GT(capturedMessages.size(), 0u); + // Total messages logged should be > 0 + EXPECT_GT(messagesLogged.load(), 0); +} + +TEST_F(LoggerHelperTest, GIVEN_callbackLoggerRegisteredFirst_WHEN_initializeDefaultLogger_THEN_callbackStillWins) { + // GIVEN - Register callback logger BEFORE the default logger (simulates + // InitCustomLogger() called before InitSDK()). The CAS guard ensures that the + // subsequent InitializeLogger(processId) is a no-op. + TestLogCapture capture; + Aws::GameLift::Server::CustomLoggerConfiguration params(TestLogCapture::Callback, &capture, Aws::GameLift::Server::LogLevel::Info); + + Aws::GameLift::GenericOutcome callbackOutcome = LoggerHelper::InitializeCallbackLogger(params); + ASSERT_TRUE(callbackOutcome.IsSuccess()); + + // Verify callback works + spdlog::info("before-default-init"); + spdlog::default_logger()->flush(); + ASSERT_FALSE(capture.messages.empty()); + EXPECT_NE(capture.messages.back().second.find("before-default-init"), std::string::npos); + + // WHEN - Try to initialize the default logger (as InitSDK would do) + RemoveDirectoryRecursive("logs"); + EnsureDirectoryExists("logs"); + Aws::GameLift::GenericOutcome defaultOutcome = LoggerHelper::InitializeLogger("test-after-callback"); + + // THEN - Should succeed (early return, no-op) and callback should still be active + EXPECT_TRUE(defaultOutcome.IsSuccess()); + + size_t countBefore = capture.messages.size(); + spdlog::info("after-default-init-attempt"); + spdlog::default_logger()->flush(); + EXPECT_GT(capture.messages.size(), countBefore); + EXPECT_NE(capture.messages.back().second.find("after-default-init-attempt"), std::string::npos); + + // No log file should have been created (callback still wins) + EXPECT_FALSE(std::ifstream("logs/gamelift-server-sdk-test-after-callback.log").good()); +} + +} // namespace Test +} // namespace Internal +} // namespace GameLift +} // namespace Aws diff --git a/gamelift-server-sdk-tests/source/aws/gamelift/server/GameLiftServerAPITest.cpp b/gamelift-server-sdk-tests/source/aws/gamelift/server/GameLiftServerAPITest.cpp index f8a3b3d..01e900c 100644 --- a/gamelift-server-sdk-tests/source/aws/gamelift/server/GameLiftServerAPITest.cpp +++ b/gamelift-server-sdk-tests/source/aws/gamelift/server/GameLiftServerAPITest.cpp @@ -13,6 +13,9 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include +#include +#include #include namespace Aws { @@ -20,7 +23,7 @@ namespace GameLift { namespace Server { namespace Test { -static const std::string sdkVersion = "5.5.0"; +static const std::string sdkVersion = "5.6.0"; TEST(GameLiftServerAPITest, GIVEN_SdkVersion_WHEN_GetSdkVersion_THEN_success) { // GIVEN @@ -35,6 +38,55 @@ TEST(GameLiftServerAPITest, GIVEN_SdkVersion_WHEN_GetSdkVersion_THEN_success) { #endif } +TEST(GameLiftServerAPITest, GIVEN_nullCallback_WHEN_initCustomLogger_THEN_returnsBadRequest) { + // Server::InitCustomLogger rejects null callbacks to prevent crashes on the first log message. + // GIVEN + Aws::GameLift::Server::CustomLoggerConfiguration params(nullptr, nullptr, Aws::GameLift::Server::LogLevel::Info); + + // WHEN + GenericOutcome outcome = Server::InitCustomLogger(params); + + // THEN + EXPECT_FALSE(outcome.IsSuccess()); + EXPECT_EQ(outcome.GetError().GetErrorType(), GAMELIFT_ERROR_TYPE::BAD_REQUEST_EXCEPTION); +} + +namespace { +void TestLogCallback(Aws::GameLift::Server::LogLevel level, const char* message, void* userData) { + auto* counter = static_cast(userData); + ++(*counter); +} +} // anonymous namespace + +TEST(GameLiftServerAPITest, GIVEN_customLoggerAlreadyRegistered_WHEN_initCustomLoggerCalledAgain_THEN_returnsAlreadyInitialized) { + // Ensure clean state for this test. + Aws::GameLift::Internal::LoggerHelper::ResetCustomLoggerRegistered(); + + // GIVEN - first InitCustomLogger with a valid callback succeeds + int callCountA = 0; + Aws::GameLift::Server::CustomLoggerConfiguration paramsA(TestLogCallback, &callCountA, Aws::GameLift::Server::LogLevel::Info); + GenericOutcome firstOutcome = Server::InitCustomLogger(paramsA); + ASSERT_TRUE(firstOutcome.IsSuccess()); + + // WHEN - second InitCustomLogger with a different valid callback + int callCountB = 0; + Aws::GameLift::Server::CustomLoggerConfiguration paramsB(TestLogCallback, &callCountB, Aws::GameLift::Server::LogLevel::Warn); + GenericOutcome secondOutcome = Server::InitCustomLogger(paramsB); + + // THEN - returns ALREADY_INITIALIZED + EXPECT_FALSE(secondOutcome.IsSuccess()); + EXPECT_EQ(secondOutcome.GetError().GetErrorType(), GAMELIFT_ERROR_TYPE::ALREADY_INITIALIZED); + + // The original callback remains active (second callback was never registered) + spdlog::info("verify original callback still active"); + spdlog::default_logger()->flush(); + EXPECT_GT(callCountA, 0); + EXPECT_EQ(callCountB, 0); + + // Cleanup + Aws::GameLift::Internal::LoggerHelper::ResetCustomLoggerRegistered(); +} + } // namespace Test } // namespace Server } // namespace GameLift diff --git a/gamelift-server-sdk/include/aws/gamelift/internal/network/WebSocketppClientWrapper.h b/gamelift-server-sdk/include/aws/gamelift/internal/network/WebSocketppClientWrapper.h index d2b8a24..fb967fc 100644 --- a/gamelift-server-sdk/include/aws/gamelift/internal/network/WebSocketppClientWrapper.h +++ b/gamelift-server-sdk/include/aws/gamelift/internal/network/WebSocketppClientWrapper.h @@ -47,6 +47,12 @@ class WebSocketppClientWrapper : public IWebSocketClientWrapper { private: const int WEBSOCKET_OPEN_HANDSHAKE_TIMEOUT_MILLIS = 20000; // 20 seconds + // Hard upper bound on how long PerformConnect will block waiting for a connection to + // open or fail. Backstop for cases where neither OnConnected nor OnError ever fires + // (e.g. a stalled TCP/TLS handshake that the websocketpp open-handshake timeout does + // not cover). Must be >= WEBSOCKET_OPEN_HANDSHAKE_TIMEOUT_MILLIS so a legitimately + // in-progress handshake can still complete before we give up on the attempt. + const int CONNECT_WAIT_TIMEOUT_MILLIS = WEBSOCKET_OPEN_HANDSHAKE_TIMEOUT_MILLIS + 5000; // 25 seconds const int SERVICE_CALL_TIMEOUT_MILLIS = 20000; // 20 seconds const int OK_STATUS_CODE = 200; const int WAIT_FOR_RECONNECT_RETRY_DELAY_SECONDS = 5; @@ -55,6 +61,11 @@ class WebSocketppClientWrapper : public IWebSocketClientWrapper { // The WebSocketpp objects this class wraps std::shared_ptr m_webSocketClient; WebSocketppClientType::connection_ptr m_connection; + // The connection PerformConnect is currently awaiting. Used so OnConnected/OnError can + // ignore stale callbacks from a connection we have already abandoned after a wait + // timeout (otherwise a late callback could falsely satisfy a later attempt's wait). + // Guarded by m_lock. + WebSocketppClientType::connection_ptr m_pendingConnection; std::unique_ptr m_socket_thread_1; std::unique_ptr m_socket_thread_2; diff --git a/gamelift-server-sdk/include/aws/gamelift/internal/util/LoggerHelper.h b/gamelift-server-sdk/include/aws/gamelift/internal/util/LoggerHelper.h index 9dacb54..5d3259c 100644 --- a/gamelift-server-sdk/include/aws/gamelift/internal/util/LoggerHelper.h +++ b/gamelift-server-sdk/include/aws/gamelift/internal/util/LoggerHelper.h @@ -11,22 +11,66 @@ */ #pragma once +#include +#include #include namespace Aws { namespace GameLift { namespace Internal { +/** + * Internal helper that manages SDK logging configuration via sink-level indirection. + * + * Design: The default spdlog logger ("multi_sink") is created once and never reassigned + * during the SDK's lifetime. Its sole sink is a dist_sink_mt, which wraps child sinks + * (stdout+file for default, or a CallbackSink for custom logging). When InitCustomLogger() is + * called after InitSDK(), the dist_sink's children are atomically swapped via set_sinks() + * — no spdlog::drop() or set_default_logger() occurs in the hot path, eliminating the + * data race on spdlog's unsynchronized default_logger_raw() pointer. + * + * Thread safety: dist_sink_mt::set_sinks() and dist_sink_mt::log() both acquire the same + * base_sink::mutex_ (vendored spdlog v1.14.0, dist_sink.h:41, base_sink-inl.h:27), + * so they are mutually exclusive. Logger::set_level() is safe because level_ is + * std::atomic (common.h:228). + * + * Call ordering: + * - InitCustomLogger before InitSDK: callback logger created with dist_sink wrapping CallbackSink. + * Subsequent InitializeLogger(processId) from InitSDK is a no-op (CAS guard check). + * - InitSDK before InitCustomLogger: default logger created with dist_sink wrapping [stdout, file]. + * InitializeCallbackLogger swaps children to [CallbackSink] via set_sinks(). + * - InitCustomLogger can be called at most once; subsequent calls return ALREADY_INITIALIZED. + */ class LoggerHelper { -#ifdef GAMELIFT_USE_STD public: - static void InitializeLogger(const std::string& process_Id); + /// Registers a custom callback logger. Safe to call before or after InitSDK(). + /// After InitSDK(), background threads may be actively logging; the sink swap is + /// serialized with in-flight log calls via the dist_sink_mt mutex. + static GenericOutcome InitializeCallbackLogger(const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters); + static bool IsCustomLoggerRegistered(); + static void ResetCustomLoggerRegistered(); + + /// Drops the SDK logger from spdlog's registry. Encapsulates the logger name + /// so callers don't need to hard-code the "multi_sink" literal. + static void DropSdkLogger(); + +#ifdef GAMELIFT_USE_STD + static GenericOutcome InitializeLogger(const std::string& process_Id); + /// Initializes the logger using logParameters. If logParameters.callback is null, this + /// overload treats it as "no custom logger configured" and falls back to the default file + /// logger — unlike the public Server::InitCustomLogger() API, which rejects null with + /// BAD_REQUEST_EXCEPTION because it represents explicit user intent. + static GenericOutcome InitializeLogger(const std::string& process_Id, const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters); #else -public: - static void InitializeLogger(const char* process_Id); + static GenericOutcome InitializeLogger(const char* process_Id); + /// Initializes the logger using logParameters. If logParameters.callback is null, this + /// overload treats it as "no custom logger configured" and falls back to the default file + /// logger — unlike the public Server::InitCustomLogger() API, which rejects null with + /// BAD_REQUEST_EXCEPTION because it represents explicit user intent. + static GenericOutcome InitializeLogger(const char* process_Id, const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters); #endif }; } // namespace Internal } // namespace GameLift -} // namespace Aws \ No newline at end of file +} // namespace Aws diff --git a/gamelift-server-sdk/include/aws/gamelift/server/CustomLoggerConfiguration.h b/gamelift-server-sdk/include/aws/gamelift/server/CustomLoggerConfiguration.h new file mode 100644 index 0000000..cee1cf7 --- /dev/null +++ b/gamelift-server-sdk/include/aws/gamelift/server/CustomLoggerConfiguration.h @@ -0,0 +1,115 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ +#pragma once + +#include + +namespace Aws { +namespace GameLift { +namespace Server { + +/** + * Log levels for the custom logging callback. + * Values are ordered by increasing severity. Trace is the most verbose; + * Fatal is the most severe. Off disables all callback dispatching. + */ +enum class LogLevel { + Trace = 0, + Debug = 1, + Info = 2, + Warn = 3, + Error = 4, + Fatal = 5, + // Values 6–99 reserved for future severity levels. + Off = 100 ///< Disables all log callback dispatching +}; + +/** + * Callback function type for custom logging. + * @param level The severity level of the log message. + * @param message The pre-formatted log message string (null-terminated). + * The pointer is only valid for the duration of the callback invocation. + * Callers that need the message after the callback returns must copy it + * (e.g., into a std::string or a queue buffer). + * The message length is unbounded -- messages may embed full service + * payloads (e.g., game session data or matchmaker data) and can be + * arbitrarily large. Do not copy the message into fixed-size buffers + * without bounds checking. + * @param userData The user-provided context pointer passed in CustomLoggerConfiguration. + * + * Thread safety: The SDK serializes callback invocations via an internal mutex + * (the callback will not be invoked from multiple threads concurrently), but it + * may be called from any thread. Implementations do not need their own + * synchronization unless they access shared external state. + * + * This serialization guarantee holds only while the SDK is running normally. + * Teardown is NOT fully synchronized: the game-session, process-terminate, and + * update-game-session handlers run on detached threads that Destroy() does not + * join, so a callback may still be invoked while Destroy() runs or shortly after + * it returns. There is therefore no point before process exit at which the SDK + * can guarantee no callback is in flight. Keep the callback, its module (DLL/ + * shared library), and userData valid until the process exits; do not free or + * unload them after Destroy() returns. Likewise, avoid retaining your own + * shared_ptr to the SDK logger across Destroy() -- logging through a retained + * reference may still invoke the callback. + * + * @note The callback is invoked synchronously on the logging thread. + * Long-running operations (network I/O, file writes, etc.) will block + * all SDK logging until the callback returns. Consider queuing messages + * for asynchronous processing if your logging backend involves I/O. + * + * @note The callback pointer, its module, and userData must remain valid until + * process exit -- not merely until Destroy() returns. See the thread + * safety notes above. + */ +using LogCallback = void(*)(LogLevel level, const char* message, void* userData); + +/** + * Parameters for configuring a custom logger. + * When a LogCallback is provided, it replaces the default stdout and file logging. + * When no LogCallback is provided (nullptr), the SDK uses its default logging behavior. + * + * @note Log configuration is immutable after InitSDK() is called. The log level + * and callback cannot be changed at runtime. To change logging behavior, + * the process must be restarted. + */ +struct AWS_GAMELIFT_API CustomLoggerConfiguration { + /** Custom log callback. Set to nullptr for default SDK logging behavior. */ + LogCallback callback = nullptr; + + /** User-provided context pointer passed to the callback. + * The SDK does not manage its lifetime. + * + * Must remain valid from InitSDK() until the process terminates -- not merely + * until Destroy() returns. See the thread safety notes on LogCallback above. + * + * @code + * Server::Destroy(); + * // Do NOT delete myLogContext here: a detached callback thread may still be + * // running and dereference it. Let the OS reclaim it at process exit, or + * // otherwise guarantee no callback can run before freeing it. + * @endcode + */ + void* userData = nullptr; + + /** Minimum log level. Messages below this level are not dispatched. */ + LogLevel minimumLogLevel = LogLevel::Info; + + CustomLoggerConfiguration() = default; + + explicit CustomLoggerConfiguration(LogCallback cb, void* ud = nullptr, LogLevel minLevel = LogLevel::Info) + : callback(cb), userData(ud), minimumLogLevel(minLevel) {} +}; + +} // namespace Server +} // namespace GameLift +} // namespace Aws diff --git a/gamelift-server-sdk/include/aws/gamelift/server/GameLiftServerAPI.h b/gamelift-server-sdk/include/aws/gamelift/server/GameLiftServerAPI.h index 3940591..64f545f 100644 --- a/gamelift-server-sdk/include/aws/gamelift/server/GameLiftServerAPI.h +++ b/gamelift-server-sdk/include/aws/gamelift/server/GameLiftServerAPI.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -81,6 +82,25 @@ Uses the provided parameters exactly as specified. To use environment variables */ AWS_GAMELIFT_API GenericOutcome InitMetrics(const Aws::GameLift::Server::MetricsParameters &metricsParameters); +/** +Initializes custom logging with the specified callback parameters. +For best results, call InitCustomLogger() BEFORE InitSDK() so that SDK initialization +diagnostics (connection setup, logger init, InitSDK errors) are routed to your +custom log callback from the very start. + +Calling InitCustomLogger() after InitSDK() is also safe — the switch is thread-safe with +respect to SDK background threads, and log output switches to the callback from that +point onward; messages logged before the call go to the default file/console logger. + +This method can be called at most once; subsequent calls return ALREADY_INITIALIZED +and do NOT change the existing logger. A null callback is rejected with +BAD_REQUEST_EXCEPTION. + +@param logParameters Parameters for configuring custom logging (callback, userData, minimumLogLevel). +@return GenericOutcome indicating success or failure. +*/ +AWS_GAMELIFT_API GenericOutcome InitCustomLogger(const Aws::GameLift::Server::CustomLoggerConfiguration &logParameters); + /** Signals Amazon GameLift Servers that the process is ready to receive GameSessions. The onStartGameSession callback will be invoked when the server is bound to a GameSession. @@ -209,6 +229,25 @@ Uses the provided parameters exactly as specified. To use environment variables */ AWS_GAMELIFT_API GenericOutcome InitMetrics(const Aws::GameLift::Server::MetricsParameters &metricsParameters); +/** +Initializes custom logging with the specified callback parameters. +For best results, call InitCustomLogger() BEFORE InitSDK() so that SDK initialization +diagnostics (connection setup, logger init, InitSDK errors) are routed to your +custom log callback from the very start. + +Calling InitCustomLogger() after InitSDK() is also safe — the switch is thread-safe with +respect to SDK background threads, and log output switches to the callback from that +point onward; messages logged before the call go to the default file/console logger. + +This method can be called at most once; subsequent calls return ALREADY_INITIALIZED +and do NOT change the existing logger. A null callback is rejected with +BAD_REQUEST_EXCEPTION. + +@param logParameters Parameters for configuring custom logging (callback, userData, minimumLogLevel). +@return GenericOutcome indicating success or failure. +*/ +AWS_GAMELIFT_API GenericOutcome InitCustomLogger(const Aws::GameLift::Server::CustomLoggerConfiguration &logParameters); + /** Signals Amazon GameLift Servers that the process is ready to receive GameSessions. The onStartGameSession callback will be invoked when the server is bound to a GameSession. diff --git a/gamelift-server-sdk/source/aws/gamelift/common/GameLiftCommonState.cpp b/gamelift-server-sdk/source/aws/gamelift/common/GameLiftCommonState.cpp index a7ec602..dc6d023 100644 --- a/gamelift-server-sdk/source/aws/gamelift/common/GameLiftCommonState.cpp +++ b/gamelift-server-sdk/source/aws/gamelift/common/GameLiftCommonState.cpp @@ -11,6 +11,9 @@ */ #include +#include +#include +#include using namespace Aws::GameLift; @@ -52,9 +55,49 @@ Aws::GameLift::Internal::GetInstanceOutcome Aws::GameLift::Internal::GameLiftCom } GenericOutcome Aws::GameLift::Internal::GameLiftCommonState::DestroyInstance() { + // The m_instance null check guards against double-destroy and prevents duplicate + // fallback logger registration if DestroyInstance() is called more than once. if (m_instance) { - delete m_instance; + // Warn BEFORE teardown swaps to the fallback logger so the message reaches the + // integrator's callback sink (not just stdout). Must precede set_default_logger(). + // Ordering rationale: this runs on the caller's thread while the callback sink is + // still installed, so no teardown race is introduced. + if (LoggerHelper::IsCustomLoggerRegistered()) { + // This warning is filtered out if the user's minimumLogLevel is above Warn — intentional, since they opted out of warn-level messages. + spdlog::warn("Destroy() called with a custom log callback registered. " + "Detached SDK handler threads may still invoke the callback after Destroy() returns; " + "keep the callback, its module, and userData valid until process exit."); + } + + // Deleting m_instance joins the health-check thread and runs destructors that may + // log, so it must precede dropping/replacing the logger. NOTE: this does NOT join + // the game-session/terminate/update handler threads, which are detached elsewhere + // in GameLiftServerState. Those threads (or SDK code they call) can therefore still + // log during/after this teardown, so this sequence cannot fully guarantee the custom + // callback is idle -- it only prevents a null default logger. See CustomLoggerConfiguration.h + // for the userData lifetime contract (valid until process exit, not until Destroy()). + + auto* instance = m_instance; m_instance = nullptr; + delete instance; + + // Install the fallback BEFORE dropping the SDK logger so that spdlog's default + // logger is never null -- a concurrent spdlog::info() from an application thread + // between the drop and set would otherwise dereference a null pointer. + // We construct the logger directly (not via stdout_color_mt) to avoid a + // duplicate-name exception if DestroyInstance() is ever called twice. + // NOTE: Under the dist_sink_mt design, the "multi_sink" logger's dist_sink still + // exists and holds shared_ptrs to its child sinks (callback or stdout/file). Dropping + // "multi_sink" from the registry and replacing the default logger releases those + // references, allowing the child sinks (and thus the CallbackSink) to be destroyed. + auto fallback = std::make_shared( + "fallback", std::make_shared()); + spdlog::set_default_logger(fallback); + LoggerHelper::DropSdkLogger(); + + // Reset the custom logger flag so a subsequent InitCustomLogger() call succeeds. + LoggerHelper::ResetCustomLoggerRegistered(); + return GenericOutcome(nullptr); } return GenericOutcome(GameLiftError(GAMELIFT_ERROR_TYPE::NOT_INITIALIZED)); diff --git a/gamelift-server-sdk/source/aws/gamelift/internal/network/WebSocketppClientWrapper.cpp b/gamelift-server-sdk/source/aws/gamelift/internal/network/WebSocketppClientWrapper.cpp index d43d97a..a0cb5d7 100644 --- a/gamelift-server-sdk/source/aws/gamelift/internal/network/WebSocketppClientWrapper.cpp +++ b/gamelift-server-sdk/source/aws/gamelift/internal/network/WebSocketppClientWrapper.cpp @@ -56,8 +56,16 @@ WebSocketppClientWrapper::WebSocketppClientWrapper(std::shared_ptrstop_perpetual()" is invoked --- // socket_thread_1: No longer waits for a connection, thread ends // socket_thread_2: Finishes handling 2nd connection, then thread ends - m_socket_thread_1 = std::unique_ptr(new std::thread([this] { m_webSocketClient->run(); })); - m_socket_thread_2 = std::unique_ptr(new std::thread([this] { m_webSocketClient->run(); })); + m_socket_thread_1 = std::unique_ptr(new std::thread([this] { + spdlog::info("socket_thread_1 entering ASIO run() loop"); + m_webSocketClient->run(); + spdlog::info("socket_thread_1 exited ASIO run() loop"); + })); + m_socket_thread_2 = std::unique_ptr(new std::thread([this] { + spdlog::info("socket_thread_2 entering ASIO run() loop"); + m_webSocketClient->run(); + spdlog::info("socket_thread_2 exited ASIO run() loop"); + })); // Set callbacks using std::placeholders::_1; @@ -117,7 +125,7 @@ GenericOutcome WebSocketppClientWrapper::Connect(const Uri &uri) { websocketpp::lib::error_code closeErrorCode; m_webSocketClient->close(oldConnection->get_handle(), websocketpp::close::status::going_away, "Websocket client reconnecting", closeErrorCode); - if (errorCode.value()) { + if (closeErrorCode.value()) { spdlog::warn("Failed to close old websocket after a connection refresh, ignoring."); } } @@ -193,6 +201,16 @@ WebSocketppClientType::connection_ptr WebSocketppClientWrapper::PerformConnect(c spdlog::info("Connection request created successfully. Waiting for connection to establish..."); } + // Record which connection we're about to await, so OnConnected/OnError can tell it + // apart from stale callbacks of previously-abandoned connections. Set before connect() + // so it is in place before any callback can fire. + { + std::lock_guard lk(m_lock); + m_pendingConnection = newConnection; + m_connectionStateChanged = false; + m_fail_error_code.clear(); + } + // Queue a new connection request (the socket thread will act on it and attempt to connect) try { m_webSocketClient->connect(newConnection); @@ -201,17 +219,37 @@ WebSocketppClientType::connection_ptr WebSocketppClientWrapper::PerformConnect(c spdlog::error("Exception while trying to connect with the webSocketClient: {}", e.what()); } spdlog::info("Connection request queued."); - // Wait for connection to succeed or fail (this makes connection synchronous) + // Wait for the connection to succeed or fail (this makes connection synchronous). + // Bounded by CONNECT_WAIT_TIMEOUT_MILLIS so we never block forever if neither + // OnConnected nor OnError ever fires (e.g. a stalled handshake the websocketpp + // open-handshake timeout does not cover). { std::unique_lock lk(m_lock); - m_cond.wait(lk, [this] { return m_connectionStateChanged; }); - spdlog::info("Connection state changed: {}", m_fail_error_code.message()); - errorCode = m_fail_error_code; - // Reset + const bool signaled = m_cond.wait_for(lk, std::chrono::milliseconds(CONNECT_WAIT_TIMEOUT_MILLIS), + [this] { return m_connectionStateChanged; }); + if (!signaled) { + spdlog::warn("Timed out after {} ms waiting for connection to open or fail; abandoning this attempt.", + CONNECT_WAIT_TIMEOUT_MILLIS); + // Surface a retryable timeout so the retry strategy advances to the next attempt. + errorCode = websocketpp::lib::error_code(websocketpp::error::open_handshake_timeout); + } else { + spdlog::info("Connection state changed: {}", m_fail_error_code.message()); + errorCode = m_fail_error_code; + } + // Stop awaiting this connection; any later callback for it will now be ignored. + m_pendingConnection = nullptr; m_connectionStateChanged = false; m_fail_error_code.clear(); } + // If we failed/abandoned the attempt, proactively close the connection so it does not + // linger or fire callbacks later. Errors here are expected (e.g. not-yet-open) and ignored. + if (errorCode.value() && newConnection) { + websocketpp::lib::error_code closeEc; + m_webSocketClient->close(newConnection->get_handle(), websocketpp::close::status::going_away, + "Abandoning failed/stalled connection attempt", closeEc); + } + if (errorCode.value()) { spdlog::error("Connection failed with errorCode: {}", errorCode.message()); } @@ -326,12 +364,37 @@ bool WebSocketppClientWrapper::IsConnected() { void WebSocketppClientWrapper::OnConnected(websocketpp::connection_hdl connection) { spdlog::info("Connected to WebSocket"); + bool isStale = false; // aquire lock and set condition variables (let main thread know connection is successful) { std::lock_guard lk(m_lock); - // set the state change variables and notify the thread that is connecting - m_connectionStateChanged = true; + // Ignore callbacks from a connection we are no longer awaiting (e.g. one abandoned + // after a wait timeout). Otherwise a late callback could falsely satisfy the wait + // of a subsequent connection attempt. + WebSocketppClientType::connection_ptr con = m_webSocketClient->get_con_from_hdl(connection); + if (!m_pendingConnection || con != m_pendingConnection) { + isStale = true; + } else { + // set the state change variables and notify the thread that is connecting + m_connectionStateChanged = true; + } } + + if (isStale) { + // The handshake for this connection completed after we stopped awaiting it (e.g. the + // connect wait timed out and PerformConnect's close() was a no-op because the socket + // was not yet open). The connection is now open with no owner, so close it here to + // avoid leaking the socket. We close outside of m_lock in case close() synchronously + // triggers callbacks that also acquire m_lock. + spdlog::warn("Ignoring OnConnected for a connection that is no longer being awaited; closing stale connection."); + websocketpp::lib::error_code closeEc; + m_webSocketClient->close(connection, websocketpp::close::status::going_away, "Closing stale connection", closeEc); + if (closeEc) { + spdlog::warn("Failed to close stale connection, ignoring: {}", closeEc.message()); + } + return; + } + m_cond.notify_one(); } @@ -342,6 +405,12 @@ void WebSocketppClientWrapper::OnError(websocketpp::connection_hdl connection) { // aquire lock and set condition variables (let main thread know an error has occurred) { std::lock_guard lk(m_lock); + // Ignore errors from a connection we are no longer awaiting (e.g. one abandoned + // after a wait timeout), so a late failure can't disturb a later attempt's wait. + if (!m_pendingConnection || con != m_pendingConnection) { + spdlog::warn("Ignoring OnError for a connection that is no longer being awaited."); + return; + } // set the state change variables and notify the thread that is connecting m_connectionStateChanged = true; m_fail_error_code = con->get_ec(); diff --git a/gamelift-server-sdk/source/aws/gamelift/internal/util/LoggerHelper.cpp b/gamelift-server-sdk/source/aws/gamelift/internal/util/LoggerHelper.cpp index 8472924..d6f8a68 100644 --- a/gamelift-server-sdk/source/aws/gamelift/internal/util/LoggerHelper.cpp +++ b/gamelift-server-sdk/source/aws/gamelift/internal/util/LoggerHelper.cpp @@ -10,43 +10,285 @@ * */ #include +#include #include #include #include +#include +#include +#include +#include +#include +#include using namespace Aws::GameLift::Internal; +static constexpr const char* LOGGER_NAME = "multi_sink"; +static constexpr size_t MAX_LOG_FILE_SIZE = 10 * 1024 * 1024; // 10MB +static constexpr size_t MAX_LOG_FILES = 5; + +// Tracks whether a custom callback logger has been registered via InitCustomLogger. +// Uses std::atomic for thread-safety consistent with the surrounding SDK code. +static std::atomic s_customLoggerRegistered{false}; + +namespace { + +class CallbackSink : public spdlog::sinks::base_sink { +public: + CallbackSink(Aws::GameLift::Server::LogCallback callback, void* userData) + : m_callback(callback), m_userData(userData) {} + +protected: + void sink_it_(const spdlog::details::log_msg& msg) override { + spdlog::memory_buf_t formatted; + spdlog::sinks::base_sink::formatter_->format(msg, formatted); + // Null-terminate the buffer in-place to avoid a std::string heap allocation. + formatted.push_back('\0'); + + Aws::GameLift::Server::LogLevel level; + switch (msg.level) { + case spdlog::level::trace: + level = Aws::GameLift::Server::LogLevel::Trace; + break; + case spdlog::level::debug: + level = Aws::GameLift::Server::LogLevel::Debug; + break; + case spdlog::level::info: + level = Aws::GameLift::Server::LogLevel::Info; + break; + case spdlog::level::warn: + level = Aws::GameLift::Server::LogLevel::Warn; + break; + case spdlog::level::err: + level = Aws::GameLift::Server::LogLevel::Error; + break; + case spdlog::level::critical: + level = Aws::GameLift::Server::LogLevel::Fatal; + break; + default: + level = Aws::GameLift::Server::LogLevel::Info; + break; + } + + try { + m_callback(level, formatted.data(), m_userData); + } catch (const std::exception& ex) { + std::cerr << "[error] Custom log callback threw an exception: " + << ex.what() << ". Message was dropped." << std::endl; + } catch (...) { + std::cerr << "[error] Custom log callback threw a non-standard exception. " + "Message was dropped." << std::endl; + } + } + + void flush_() override {} + +private: + Aws::GameLift::Server::LogCallback m_callback; + void* m_userData; +}; + +spdlog::level::level_enum MapLogLevel(Aws::GameLift::Server::LogLevel level) { + switch (level) { + case Aws::GameLift::Server::LogLevel::Trace: + return spdlog::level::trace; + case Aws::GameLift::Server::LogLevel::Debug: + return spdlog::level::debug; + case Aws::GameLift::Server::LogLevel::Warn: + return spdlog::level::warn; + case Aws::GameLift::Server::LogLevel::Error: + return spdlog::level::err; + case Aws::GameLift::Server::LogLevel::Fatal: + return spdlog::level::critical; + case Aws::GameLift::Server::LogLevel::Off: + return spdlog::level::off; + case Aws::GameLift::Server::LogLevel::Info: + default: + return spdlog::level::info; + } +} + +/// Retrieves the dist_sink_mt from the current default logger, if the logger exists +/// and its first (and only) sink is a dist_sink_mt. Returns nullptr otherwise. +std::shared_ptr GetDistSink() { + auto logger = spdlog::default_logger(); + if (!logger || logger->sinks().empty()) { + return nullptr; + } + return std::dynamic_pointer_cast(logger->sinks().front()); +} + +/// Creates the "multi_sink" logger with a dist_sink_mt wrapping the given children, +/// registers it in spdlog's registry, and sets it as the default logger. +/// The dist_sink_mt provides the indirection layer: log() and set_sinks() are +/// mutually exclusive under the same std::mutex (base_sink-inl.h:27, dist_sink.h:41 +/// in vendored spdlog v1.14.0), so child sinks can be swapped atomically while +/// background threads are concurrently logging through the same logger instance. +std::shared_ptr CreateDistSinkLogger( + std::vector> children, + spdlog::level::level_enum level, + spdlog::level::level_enum flushLevel) { + + auto distSink = std::make_shared(std::move(children)); + auto logger = std::make_shared(LOGGER_NAME, spdlog::sinks_init_list{distSink}); + logger->set_level(level); + logger->flush_on(flushLevel); + spdlog::set_default_logger(logger); + return distSink; +} + +} // anonymous namespace + +bool LoggerHelper::IsCustomLoggerRegistered() { + return s_customLoggerRegistered.load(std::memory_order_acquire); +} + +void LoggerHelper::ResetCustomLoggerRegistered() { + s_customLoggerRegistered.store(false, std::memory_order_release); +} + +void LoggerHelper::DropSdkLogger() { + spdlog::drop(LOGGER_NAME); +} + +Aws::GameLift::GenericOutcome LoggerHelper::InitializeCallbackLogger(const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters) { + // Atomic CAS ensures only one concurrent caller succeeds in registering the custom logger. + bool expected = false; + if (!s_customLoggerRegistered.compare_exchange_strong(expected, true, std::memory_order_acq_rel)) { + return Aws::GameLift::GenericOutcome(Aws::GameLift::GameLiftError(Aws::GameLift::GAMELIFT_ERROR_TYPE::ALREADY_INITIALIZED, + "Custom logger has already been initialized via InitCustomLogger().")); + } + + try { + auto callbackSink = std::make_shared(logParameters.callback, logParameters.userData); + callbackSink->set_formatter( + std::unique_ptr(new spdlog::pattern_formatter("%v", spdlog::pattern_time_type::local, ""))); + + auto distSink = GetDistSink(); + if (distSink) { + // The default logger already exists (InitSDK ran first and called InitializeLogger(processId)). + // Swap the dist_sink's children from [stdout, file] to [callbackSink]. + // THREAD SAFETY: dist_sink_mt::set_sinks() acquires base_sink::mutex_ + // (dist_sink.h:41, vendored spdlog v1.14.0). dist_sink_mt::log() (called by any + // concurrent spdlog::info() etc.) also acquires the SAME mutex via base_sink::log() + // (base_sink-inl.h:27). Therefore set_sinks() and log() are mutually exclusive — + // no data race, no use-after-free on the old child sinks. + distSink->set_sinks({callbackSink}); + + // Update the logger level. spdlog::logger::level_ is std::atomic + // (common.h:228, vendored spdlog v1.14.0), so set_level() is safe to call + // concurrently with should_log() checks on other threads. + spdlog::default_logger()->set_level(MapLogLevel(logParameters.minimumLogLevel)); + } else { + // No logger exists yet (InitCustomLogger called before InitSDK). + // Create the dist_sink logger with only the callback sink as its child. + // No race here — if InitSDK hasn't run, no background threads are logging yet, + // and the CAS above prevents concurrent InitializeCallbackLogger calls. + CreateDistSinkLogger({callbackSink}, MapLogLevel(logParameters.minimumLogLevel), spdlog::level::off); + } + // Note: flush_on is not set/changed for callback path because CallbackSink has no + // internal buffer — messages are dispatched to the callback immediately in sink_it_(). + + return Aws::GameLift::GenericOutcome(nullptr); + } catch (const std::exception& ex) { + // Rollback the flag so a retry can succeed after the transient failure is resolved. + s_customLoggerRegistered.store(false, std::memory_order_release); + std::string errorMessage = "Failed to initialize custom logger: " + std::string(ex.what()); + std::cerr << "[error] " << errorMessage << std::endl; + return Aws::GameLift::GenericOutcome(Aws::GameLift::GameLiftError(Aws::GameLift::GAMELIFT_ERROR_TYPE::GAMELIFT_SERVER_NOT_INITIALIZED, +#ifdef GAMELIFT_USE_STD + errorMessage)); +#else + errorMessage.c_str())); +#endif + } +} + #ifdef GAMELIFT_USE_STD -void LoggerHelper::InitializeLogger(const std::string& process_Id) { - auto console_sink = std::make_shared(); - std::string serverSdkLog = "logs/gamelift-server-sdk-"; - serverSdkLog.append(process_Id).append(".log"); - auto file_sink = std::make_shared(serverSdkLog, 10485760, 5); +Aws::GameLift::GenericOutcome LoggerHelper::InitializeLogger(const std::string& process_Id) { + // If a custom logger was already registered via InitCustomLogger, skip default logger initialization. + // The callback logger's dist_sink is already in place; InitSDK's default sinks must NOT clobber it. + if (s_customLoggerRegistered.load(std::memory_order_acquire)) { + return Aws::GameLift::GenericOutcome(nullptr); + } + if (spdlog::get(LOGGER_NAME)) { + spdlog::warn("Logger already initialized, skipping duplicate initialization"); + return Aws::GameLift::GenericOutcome(nullptr); + } + try { + auto console_sink = std::make_shared(); + std::string serverSdkLog = "logs/gamelift-server-sdk-"; + serverSdkLog.append(process_Id).append(".log"); + auto file_sink = std::make_shared(serverSdkLog, MAX_LOG_FILE_SIZE, MAX_LOG_FILES); - console_sink->set_pattern("%^[%Y-%m-%d %H:%M:%S] [%l] %v%$"); - file_sink->set_pattern("[%Y-%m-%d %H:%M:%S] [%l] %v"); + console_sink->set_pattern("%^[%Y-%m-%d %H:%M:%S.%e] [%l] [tid %t] %v%$"); + file_sink->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [%l] [tid %t] %v"); - spdlog::logger logger("multi_sink", { console_sink, file_sink }); - logger.set_level(spdlog::level::info); - logger.flush_on(spdlog::level::info); + // Create the logger with a dist_sink_mt wrapping the console and file sinks. + // The dist_sink provides sink-level indirection: if InitCustomLogger() is called later + // (after InitSDK), it can atomically swap children via set_sinks() without replacing + // the logger instance — eliminating the data race on spdlog's default_logger_raw() + // pointer that the old drop()+set_default_logger() approach caused. + CreateDistSinkLogger({console_sink, file_sink}, spdlog::level::info, spdlog::level::info); - spdlog::set_default_logger(std::make_shared(logger)); + return Aws::GameLift::GenericOutcome(nullptr); + } catch (const std::exception& ex) { + std::string errorMessage = "Failed to initialize logger: " + std::string(ex.what()); + std::cerr << "[error] " << errorMessage << std::endl; + return Aws::GameLift::GenericOutcome(Aws::GameLift::GameLiftError(Aws::GameLift::GAMELIFT_ERROR_TYPE::GAMELIFT_SERVER_NOT_INITIALIZED, + errorMessage)); + } } #else -void LoggerHelper::InitializeLogger(const char* process_Id) { - auto console_sink = std::make_shared(); - std::string serverSdkLog = "logs/gamelift-server-sdk-"; - serverSdkLog.append(process_Id).append(".log"); - auto file_sink = std::make_shared(serverSdkLog, 10485760, 5); +Aws::GameLift::GenericOutcome LoggerHelper::InitializeLogger(const char* process_Id) { + // If a custom logger was already registered via InitCustomLogger, skip default logger initialization. + // The callback logger's dist_sink is already in place; InitSDK's default sinks must NOT clobber it. + if (s_customLoggerRegistered.load(std::memory_order_acquire)) { + return Aws::GameLift::GenericOutcome(nullptr); + } + if (spdlog::get(LOGGER_NAME)) { + spdlog::warn("Logger already initialized, skipping duplicate initialization"); + return Aws::GameLift::GenericOutcome(nullptr); + } + try { + auto console_sink = std::make_shared(); + std::string serverSdkLog = "logs/gamelift-server-sdk-"; + serverSdkLog.append(process_Id).append(".log"); + auto file_sink = std::make_shared(serverSdkLog, MAX_LOG_FILE_SIZE, MAX_LOG_FILES); - console_sink->set_pattern("%^[%Y-%m-%d %H:%M:%S] [%l] %v%$"); - file_sink->set_pattern("[%Y-%m-%d %H:%M:%S] [%l] %v"); + console_sink->set_pattern("%^[%Y-%m-%d %H:%M:%S.%e] [%l] [tid %t] %v%$"); + file_sink->set_pattern("[%Y-%m-%d %H:%M:%S.%e] [%l] [tid %t] %v"); - spdlog::logger logger("multi_sink", { console_sink, file_sink }); - logger.set_level(spdlog::level::info); - logger.flush_on(spdlog::level::info); + // Create the logger with a dist_sink_mt wrapping the console and file sinks. + // See the GAMELIFT_USE_STD overload above for the full concurrency rationale. + CreateDistSinkLogger({console_sink, file_sink}, spdlog::level::info, spdlog::level::info); - spdlog::set_default_logger(std::make_shared(logger)); + return Aws::GameLift::GenericOutcome(nullptr); + } catch (const std::exception& ex) { + std::string errorMessage = "Failed to initialize logger: " + std::string(ex.what()); + std::cerr << "[error] " << errorMessage << std::endl; + return Aws::GameLift::GenericOutcome(Aws::GameLift::GameLiftError(Aws::GameLift::GAMELIFT_ERROR_TYPE::GAMELIFT_SERVER_NOT_INITIALIZED, + errorMessage.c_str())); + } } #endif +#ifdef GAMELIFT_USE_STD +Aws::GameLift::GenericOutcome LoggerHelper::InitializeLogger(const std::string& process_Id, const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters) { + // If a callback is provided, delegate to the callback logger path. + if (logParameters.callback != nullptr) { + return InitializeCallbackLogger(logParameters); + } + // No callback — fall through to the default file/stdout logger. + return InitializeLogger(process_Id); +} +#else +Aws::GameLift::GenericOutcome LoggerHelper::InitializeLogger(const char* process_Id, const Aws::GameLift::Server::CustomLoggerConfiguration& logParameters) { + // If a callback is provided, delegate to the callback logger path. + if (logParameters.callback != nullptr) { + return InitializeCallbackLogger(logParameters); + } + // No callback — fall through to the default file/stdout logger. + return InitializeLogger(process_Id); +} +#endif diff --git a/gamelift-server-sdk/source/aws/gamelift/server/GameLiftServerAPI.cpp b/gamelift-server-sdk/source/aws/gamelift/server/GameLiftServerAPI.cpp index 769d2d9..5afdcc4 100644 --- a/gamelift-server-sdk/source/aws/gamelift/server/GameLiftServerAPI.cpp +++ b/gamelift-server-sdk/source/aws/gamelift/server/GameLiftServerAPI.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -24,7 +25,7 @@ using namespace Aws::GameLift; -static const std::string sdkVersion = "5.5.0"; +static const std::string sdkVersion = "5.6.0"; #ifdef GAMELIFT_USE_STD Aws::GameLift::AwsStringOutcome Server::GetSdkVersion() { return AwsStringOutcome(sdkVersion); } @@ -32,7 +33,10 @@ Aws::GameLift::AwsStringOutcome Server::GetSdkVersion() { return AwsStringOutcom Server::InitSDKOutcome Server::InitSDK() { return InitSDK(Aws::GameLift::Server::Model::ServerParameters()); } Server::InitSDKOutcome Server::InitSDK(const Aws::GameLift::Server::Model::ServerParameters &serverParameters) { - Internal::LoggerHelper::InitializeLogger(serverParameters.GetProcessId()); + GenericOutcome loggerOutcome = Internal::LoggerHelper::InitializeLogger(serverParameters.GetProcessId()); + if (!loggerOutcome.IsSuccess()) { + return InitSDKOutcome(loggerOutcome.GetError()); + } spdlog::info("Initializing GameLift SDK"); // Initialize the WebSocketWrapper std::shared_ptr webSocketClientWrapper; @@ -54,6 +58,8 @@ Server::InitSDKOutcome Server::InitSDK(const Aws::GameLift::Server::Model::Serve if (globalProcessor != nullptr) { initOutcome.GetResult()->SetGlobalProcessor(globalProcessor); } + } else { + spdlog::error("Failed to create server state instance"); } return initOutcome; } @@ -192,7 +198,10 @@ Aws::GameLift::AwsStringOutcome Server::GetSdkVersion() { return AwsStringOutcom GenericOutcome Server::InitSDK() { return InitSDK(Aws::GameLift::Server::Model::ServerParameters()); } GenericOutcome Server::InitSDK(const Aws::GameLift::Server::Model::ServerParameters &serverParameters) { - Internal::LoggerHelper::InitializeLogger(serverParameters.GetProcessId()); + GenericOutcome loggerOutcome = Internal::LoggerHelper::InitializeLogger(serverParameters.GetProcessId()); + if (!loggerOutcome.IsSuccess()) { + return loggerOutcome; + } spdlog::info("Initializing server SDK"); // Initialize the WebSocketWrapper Internal::InitSDKOutcome initOutcome = @@ -211,6 +220,9 @@ GenericOutcome Server::InitSDK(const Aws::GameLift::Server::Model::ServerParamet if (globalProcessor != nullptr) { initOutcome.GetResult()->SetGlobalProcessor(globalProcessor); } + } else { + spdlog::error("Failed to create server state instance"); + return GenericOutcome(initOutcome.GetError()); } return GenericOutcome(nullptr); } @@ -363,7 +375,7 @@ DescribePlayerSessionsOutcome Server::DescribePlayerSessions(const Aws::GameLift GenericOutcome Server::Destroy() { Aws::GameLift::Metrics::MetricsTerminate(); spdlog::info("Metrics terminated"); - return Internal::GameLiftCommonState::DestroyInstance(); + return Internal::GameLiftCommonState::DestroyInstance(); } GetComputeCertificateOutcome Server::GetComputeCertificate() { @@ -456,3 +468,14 @@ GenericOutcome Server::InitMetrics(const Aws::GameLift::Server::MetricsParameter return GenericOutcome(nullptr); } + +GenericOutcome Server::InitCustomLogger(const Aws::GameLift::Server::CustomLoggerConfiguration &logParameters) { + // Reject null callback early — passing null would crash on the first log message. + if (logParameters.callback == nullptr) { + return GenericOutcome(GameLiftError(GAMELIFT_ERROR_TYPE::BAD_REQUEST_EXCEPTION, + "InitCustomLogger requires a non-null callback.")); + } + + // InitializeCallbackLogger's compare_exchange handles the already-registered case atomically. + return Internal::LoggerHelper::InitializeCallbackLogger(logParameters); +} diff --git a/release-notes/5.6.0-release-notes.txt b/release-notes/5.6.0-release-notes.txt new file mode 100644 index 0000000..cd4d6e1 --- /dev/null +++ b/release-notes/5.6.0-release-notes.txt @@ -0,0 +1,3 @@ +- Adds the InitCustomLogger() server SDK action to route server SDK log output to a custom logging callback, for example the Unreal Engine logging system. +- Improves logging with thread IDs in log messages and more robust logger initialization. +- Fixes SDK initialization to report failures correctly, and prevents the server from waiting indefinitely on a broken WebSocket reconnection.