fix(threadpool): Prevent FreezeDetector crash with shorter timeouts

Root cause: ThreadPool::WaitForCompletion could block world thread for
35+ seconds (5s + 30s), leaving insufficient buffer before FreezeDetector
triggers at 60 seconds. Combined with other World::Update operations,
total time exceeded 60s causing forced crash.

Changes:
- BotWorldSessionMgr: Reduce wait timeouts from 5s+30s to 2s+8s (10s max)
- ThreadPool::WaitForCompletion: Add hard cap of 15 seconds regardless
  of caller-specified timeout
- ThreadPool.h: Change default timeout from milliseconds::max() to 10s

This ensures world thread never blocks more than 15 seconds in
WaitForCompletion, leaving 45+ seconds buffer for FreezeDetector.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
This commit is contained in:
agatho
2026-02-02 13:23:11 -03:00
committed by luis
co-authored by Claude Opus 4.5
parent 8ce4578e85
commit 7c68c7b4d0
3 changed files with 46 additions and 36 deletions
@@ -755,6 +755,11 @@ bool ThreadPool::WaitForCompletion(::std::chrono::milliseconds timeout)
{
auto start = ::std::chrono::steady_clock::now();
// CRITICAL SAFETY: Hard cap at 15 seconds to prevent FreezeDetector crash (60s)
// Even if caller passes a larger timeout, we refuse to wait longer
constexpr auto HARD_MAX_TIMEOUT = ::std::chrono::milliseconds(15000);
auto effectiveTimeout = ::std::min(timeout, HARD_MAX_TIMEOUT);
while (true)
{
// Check if all queues are empty
@@ -787,9 +792,9 @@ bool ThreadPool::WaitForCompletion(::std::chrono::milliseconds timeout)
if (allEmpty && allTasksFinished)
return true;
// Check timeout
// Check timeout - use effective timeout with hard cap
auto now = ::std::chrono::steady_clock::now();
if (::std::chrono::duration_cast<::std::chrono::milliseconds>(now - start) >= timeout)
if (::std::chrono::duration_cast<::std::chrono::milliseconds>(now - start) >= effectiveTimeout)
return false;
::std::this_thread::sleep_for(::std::chrono::milliseconds(10));
@@ -615,7 +615,8 @@ public:
* @param timeout Maximum time to wait
* @return true if all tasks completed, false if timeout
*/
bool WaitForCompletion(::std::chrono::milliseconds timeout = ::std::chrono::milliseconds::max());
// CRITICAL: Default timeout is 10 seconds, hard cap at 15 seconds to prevent FreezeDetector crash
bool WaitForCompletion(::std::chrono::milliseconds timeout = ::std::chrono::milliseconds(10000));
/**
* @brief Initiate graceful shutdown
@@ -1019,50 +1019,54 @@ void BotWorldSessionMgr::UpdateSessions(uint32 diff)
auto startWait = ::std::chrono::steady_clock::now();
// Wait indefinitely for completion - we CANNOT proceed while workers cast spells
// Using max timeout to effectively block until all tasks complete
// WaitForCompletion now correctly waits for BOTH queued AND executing tasks
bool completed = Performance::GetThreadPool().WaitForCompletion(::std::chrono::milliseconds(5000));
// CRITICAL FIX (FreezeDetector 60s crash): Use much shorter timeouts!
// FreezeDetector triggers at 60s. We must leave plenty of buffer for:
// - Other World::Update operations after UpdateSessions
// - Any unexpected delays
// Total wait should be MAX 10 seconds (leaving 50s buffer)
constexpr auto INITIAL_WAIT = ::std::chrono::milliseconds(2000);
constexpr auto EXTENDED_WAIT = ::std::chrono::milliseconds(8000);
constexpr auto MAX_TOTAL_WAIT = ::std::chrono::milliseconds(10000);
bool completed = Performance::GetThreadPool().WaitForCompletion(INITIAL_WAIT);
auto waitDuration = ::std::chrono::duration_cast<::std::chrono::milliseconds>(
::std::chrono::steady_clock::now() - startWait);
if (!completed && waitDuration < MAX_TOTAL_WAIT)
{
// Tasks still running - log and try extended wait
TC_LOG_WARN("module.playerbot.session",
"ThreadPool tasks still running after {}ms ({} queued, {} in-flight) - trying extended wait",
waitDuration.count(), queuedTasks, inFlightTasks);
// Calculate remaining time for extended wait
auto remaining = MAX_TOTAL_WAIT - waitDuration;
auto extendedTimeout = ::std::min(EXTENDED_WAIT, remaining);
completed = Performance::GetThreadPool().WaitForCompletion(extendedTimeout);
waitDuration = ::std::chrono::duration_cast<::std::chrono::milliseconds>(
::std::chrono::steady_clock::now() - startWait);
}
if (!completed)
{
// This should rarely happen - log as error if tasks take > 5 seconds
// CRITICAL: Still not completed - proceed anyway to prevent FreezeDetector crash
size_t finalQueued = Performance::GetThreadPool().GetQueuedTasks();
size_t finalInFlight = Performance::GetThreadPool().GetInFlightTasks();
size_t activeThreads = Performance::GetThreadPool().GetActiveThreads();
TC_LOG_ERROR("module.playerbot.session",
"ThreadPool tasks still running after 5000ms ({} queued, {} in-flight) - trying extended wait",
queuedTasks, inFlightTasks);
"ThreadPool wait timeout after {}ms! {} queued, {} in-flight, {} active workers. "
"PROCEEDING to prevent FreezeDetector crash - some bot updates may be incomplete!",
waitDuration.count(), finalQueued, finalInFlight, activeThreads);
// CRITICAL FIX (FreezeDetector crash): NEVER use milliseconds::max()!
// That would block the world thread forever if there's a counter leak bug.
// Instead, use a reasonable maximum timeout (30 seconds) and then proceed anyway.
// The ThreadPool counter leak bug has been fixed, but this safeguard remains.
constexpr auto MAX_EXTENDED_WAIT = ::std::chrono::milliseconds(30000);
bool extendedCompleted = Performance::GetThreadPool().WaitForCompletion(MAX_EXTENDED_WAIT);
if (!extendedCompleted)
{
// CRITICAL: Still not completed after 30s - likely a bug or deadlock
// Log detailed diagnostic info and proceed anyway to prevent FreezeDetector crash
size_t finalQueued = Performance::GetThreadPool().GetQueuedTasks();
size_t finalInFlight = Performance::GetThreadPool().GetInFlightTasks();
size_t activeThreads = Performance::GetThreadPool().GetActiveThreads();
TC_LOG_FATAL("module.playerbot.session",
"ThreadPool DEADLOCK DETECTED! After 35s total wait: {} queued, {} in-flight, {} active workers. "
"PROCEEDING ANYWAY to prevent FreezeDetector crash - may cause instability!",
finalQueued, finalInFlight, activeThreads);
}
else
{
TC_LOG_WARN("module.playerbot.session", "ThreadPool finally completed after extended wait");
}
canProcessLogouts = false;
}
else if (waitDuration.count() > 100)
else if (waitDuration.count() > 500)
{
// Log if wait was notably long (>100ms)
// Log if wait was notably long (>500ms)
TC_LOG_DEBUG("module.playerbot.session",
"ThreadPool wait took {}ms for {} tasks", waitDuration.count(), queuedTasks);
}