From 65b6e15ea1ff87db9e50463f93d74c4d6de340d3 Mon Sep 17 00:00:00 2001 From: EricksOliveira Date: Fri, 18 Oct 2024 09:27:46 -0300 Subject: [PATCH 1/3] Fix Crash - OnBotLogin 1. Improvements in error handling: Added detailed logs for cases where botAI or master are null, allowing better failure tracking. 2. Additional null pointer checks: Added checks to ensure that the botAI and master are valid before performing actions dependent on these objects, preventing potential crashes. 3. Optimization in bot login logic: Revised the bot input flow to ensure it is added to the group appropriately depending on its relationship to the master. Added logic for handling different types of groups (raid, LFG, etc.), including the possibility of automatic conversion to raid if necessary. --- src/PlayerbotMgr.cpp | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/PlayerbotMgr.cpp b/src/PlayerbotMgr.cpp index 34e107d1..3cae1645 100644 --- a/src/PlayerbotMgr.cpp +++ b/src/PlayerbotMgr.cpp @@ -100,6 +100,8 @@ void PlayerbotHolder::HandlePlayerBotLoginCallback(PlayerbotLoginQueryHolder con Player* bot = botSession->GetPlayer(); if (!bot) { + // Log para debug + LOG_ERROR("mod-playerbots", "Bot player could not be loaded for account ID: {}", botAccountId); botSession->LogoutPlayer(true); delete botSession; botLoading.erase(holder.GetGuid()); @@ -108,6 +110,14 @@ void PlayerbotHolder::HandlePlayerBotLoginCallback(PlayerbotLoginQueryHolder con uint32 masterAccount = holder.GetMasterAccountId(); WorldSession* masterSession = masterAccount ? sWorld->FindSession(masterAccount) : nullptr; + + // Check if masterSession->GetPlayer() is valid + Player* masterPlayer = masterSession ? masterSession->GetPlayer() : nullptr; + if (masterSession && !masterPlayer) + { + LOG_ERROR("mod-playerbots", "Master session found but no player is associated for master account ID: {}", masterAccount); + } + std::ostringstream out; bool allowed = false; if (botAccountId == masterAccount) @@ -115,7 +125,7 @@ void PlayerbotHolder::HandlePlayerBotLoginCallback(PlayerbotLoginQueryHolder con allowed = true; } else if (masterSession && sPlayerbotAIConfig->allowGuildBots && bot->GetGuildId() != 0 && - bot->GetGuildId() == masterSession->GetPlayer()->GetGuildId()) + bot->GetGuildId() == masterPlayer->GetGuildId()) { allowed = true; } @@ -129,10 +139,14 @@ void PlayerbotHolder::HandlePlayerBotLoginCallback(PlayerbotLoginQueryHolder con out << "Failure: You are not allowed to control bot " << bot->GetName().c_str(); } - if (allowed && masterSession) + if (allowed && masterSession && masterPlayer) { - Player* player = masterSession->GetPlayer(); - PlayerbotMgr* mgr = GET_PLAYERBOT_MGR(player); + PlayerbotMgr* mgr = GET_PLAYERBOT_MGR(masterPlayer); + if (!mgr) + { + LOG_ERROR("mod-playerbots", "PlayerbotMgr not found for master player with GUID: {}", masterPlayer->GetGUID().GetRawValue()); + } + uint32 count = mgr->GetPlayerbotsCount(); uint32 cls_count = mgr->GetPlayerbotsCountByClass(bot->getClass()); if (count >= sPlayerbotAIConfig->maxAddedBots) @@ -428,14 +442,17 @@ void PlayerbotHolder::OnBotLogin(Player* const bot) PlayerbotAI* botAI = GET_PLAYERBOT_AI(bot); if (!botAI) { + // Log a warning here to indicate that the botAI is null + LOG_ERROR("mod-playerbots", "PlayerbotAI is null for bot with GUID: {}", bot->GetGUID().GetRawValue()); return; } + Player* master = botAI->GetMaster(); - if (master) + if (!master) { - ObjectGuid masterGuid = master->GetGUID(); - if (master->GetGroup() && !master->GetGroup()->IsLeader(masterGuid)) - master->GetGroup()->ChangeLeader(masterGuid); + // Log a warning to indicate that the master is null + LOG_ERROR("mod-playerbots", "Master is null for bot with GUID: {}", bot->GetGUID().GetRawValue()); + return; } Group* group = bot->GetGroup(); From 8f2455b7663c935fb33d3552bb1371390d1106f8 Mon Sep 17 00:00:00 2001 From: EricksOliveira Date: Fri, 18 Oct 2024 14:37:54 -0300 Subject: [PATCH 2/3] Update PlayerbotCommandServer.cpp Protection against race conditions: Added a global mutex (session_mutex) to synchronize access to the session() function when multiple threads are running. This avoids concurrency problems in environments with many simultaneous connections. Thread pool usage: Replaced manual thread creation with boost::thread with a thread pool using boost::asio::thread_pool. The thread pool improves resource management and limits the number of simultaneous threads, optimizing performance. Checking for socket errors: Added check to ensure the socket is not null or closed before trying to read it. Implemented detailed error handling for readings and writings, including detailed logs to facilitate debugging. Secure socket closure: Now sockets are correctly closed at the end of the session, even in the event of an error, preventing resource leaks. --- src/PlayerbotCommandServer.cpp | 52 ++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/PlayerbotCommandServer.cpp b/src/PlayerbotCommandServer.cpp index 482c2348..c08f1d05 100644 --- a/src/PlayerbotCommandServer.cpp +++ b/src/PlayerbotCommandServer.cpp @@ -9,8 +9,10 @@ #include #include #include +#include #include #include +#include #include "IoContext.h" #include "Playerbots.h" @@ -18,21 +20,39 @@ using boost::asio::ip::tcp; typedef boost::shared_ptr socket_ptr; +std::mutex session_mutex; // Global mutex to protect sessions +boost::asio::thread_pool pool(4); // 4-thread thread pool + bool ReadLine(socket_ptr sock, std::string* buffer, std::string* line) { - // Do the real reading from fd until buffer has '\n'. + // Check if the socket is valid before using it + if (!sock || !sock->is_open()) + { + LOG_ERROR("playerbots", "Invalid or closed socket."); + return false; + } + + // Does the actual reading until the buffer has a '\n' std::string::iterator pos; while ((pos = find(buffer->begin(), buffer->end(), '\n')) == buffer->end()) { char buf[1025]; boost::system::error_code error; - size_t n = sock->read_some(boost::asio::buffer(buf), error); - if (n == -1 || error == boost::asio::error::eof) - return false; - else if (error) - throw boost::system::system_error(error); // Some other error. - buf[n] = 0; + // Read socket data + size_t n = sock->read_some(boost::asio::buffer(buf), error); + if (n == 0 || error == boost::asio::error::eof) + { + LOG_INFO("playerbots", "Connection closed by peer."); + return false; + } + else if (error) + { + LOG_ERROR("playerbots", "Socket read error: {}", error.message()); + return false; // Returns false in case of error. + } + + buf[n] = 0; // Ensures the buffer ends with null *buffer += buf; } @@ -45,6 +65,8 @@ void session(socket_ptr sock) { try { + std::lock_guard guard(session_mutex); // Protect session with mutex + std::string buffer, request; while (ReadLine(sock, &buffer, &request)) { @@ -55,7 +77,19 @@ void session(socket_ptr sock) } catch (std::exception& e) { - LOG_ERROR("playerbots", "{}", e.what()); + LOG_ERROR("playerbots", "Session error: {}", e.what()); + } + + // Make sure to close the socket at the end of the session + if (sock && sock->is_open()) + { + boost::system::error_code ec; + sock->shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); + sock->close(ec); + if (ec) + { + LOG_ERROR("playerbots", "Error closing socket: {}", ec.message()); + } } } @@ -66,7 +100,7 @@ void server(Acore::Asio::IoContext& io_service, short port) { socket_ptr sock(new tcp::socket(io_service)); a.accept(*sock); - boost::thread t(boost::bind(session, sock)); + boost::asio::post(pool, boost::bind(session, sock)); // Use thread pool instead of creating new threads } } From eabad44d4bcc45a255fb7bdda409e6f7a1a47e64 Mon Sep 17 00:00:00 2001 From: EricksOliveira Date: Fri, 18 Oct 2024 14:40:25 -0300 Subject: [PATCH 3/3] Revert "Update PlayerbotCommandServer.cpp" This reverts commit 8f2455b7663c935fb33d3552bb1371390d1106f8. --- src/PlayerbotCommandServer.cpp | 46 +++++----------------------------- 1 file changed, 6 insertions(+), 40 deletions(-) diff --git a/src/PlayerbotCommandServer.cpp b/src/PlayerbotCommandServer.cpp index c08f1d05..482c2348 100644 --- a/src/PlayerbotCommandServer.cpp +++ b/src/PlayerbotCommandServer.cpp @@ -9,10 +9,8 @@ #include #include #include -#include #include #include -#include #include "IoContext.h" #include "Playerbots.h" @@ -20,39 +18,21 @@ using boost::asio::ip::tcp; typedef boost::shared_ptr socket_ptr; -std::mutex session_mutex; // Global mutex to protect sessions -boost::asio::thread_pool pool(4); // 4-thread thread pool - bool ReadLine(socket_ptr sock, std::string* buffer, std::string* line) { - // Check if the socket is valid before using it - if (!sock || !sock->is_open()) - { - LOG_ERROR("playerbots", "Invalid or closed socket."); - return false; - } - - // Does the actual reading until the buffer has a '\n' + // Do the real reading from fd until buffer has '\n'. std::string::iterator pos; while ((pos = find(buffer->begin(), buffer->end(), '\n')) == buffer->end()) { char buf[1025]; boost::system::error_code error; - - // Read socket data size_t n = sock->read_some(boost::asio::buffer(buf), error); - if (n == 0 || error == boost::asio::error::eof) - { - LOG_INFO("playerbots", "Connection closed by peer."); + if (n == -1 || error == boost::asio::error::eof) return false; - } else if (error) - { - LOG_ERROR("playerbots", "Socket read error: {}", error.message()); - return false; // Returns false in case of error. - } + throw boost::system::system_error(error); // Some other error. - buf[n] = 0; // Ensures the buffer ends with null + buf[n] = 0; *buffer += buf; } @@ -65,8 +45,6 @@ void session(socket_ptr sock) { try { - std::lock_guard guard(session_mutex); // Protect session with mutex - std::string buffer, request; while (ReadLine(sock, &buffer, &request)) { @@ -77,19 +55,7 @@ void session(socket_ptr sock) } catch (std::exception& e) { - LOG_ERROR("playerbots", "Session error: {}", e.what()); - } - - // Make sure to close the socket at the end of the session - if (sock && sock->is_open()) - { - boost::system::error_code ec; - sock->shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); - sock->close(ec); - if (ec) - { - LOG_ERROR("playerbots", "Error closing socket: {}", ec.message()); - } + LOG_ERROR("playerbots", "{}", e.what()); } } @@ -100,7 +66,7 @@ void server(Acore::Asio::IoContext& io_service, short port) { socket_ptr sock(new tcp::socket(io_service)); a.accept(*sock); - boost::asio::post(pool, boost::bind(session, sock)); // Use thread pool instead of creating new threads + boost::thread t(boost::bind(session, sock)); } }