From b0bc1fa310bd60d45d1744073a8b8523dfe6203e Mon Sep 17 00:00:00 2001 From: Ari Vuollet Date: Sun, 29 Jan 2023 21:25:29 +0200 Subject: [PATCH] Fix error when joining exited threads The internal thread handles were cleared prematurely when attempting to join them. The handles should be also cleared when trying to kill already exited threads. --- Source/Engine/Content/Loading/ContentLoadingManager.cpp | 3 +-- Source/Engine/Platform/Base/ThreadBase.cpp | 4 +++- Source/Engine/Platform/Unix/UnixThread.cpp | 1 + Source/Engine/Platform/Win32/Win32Thread.cpp | 1 + Source/Engine/Threading/JobSystem.cpp | 3 +-- Source/Engine/Threading/ThreadPool.cpp | 5 +---- 6 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Source/Engine/Content/Loading/ContentLoadingManager.cpp b/Source/Engine/Content/Loading/ContentLoadingManager.cpp index c9680441b..7fd63a217 100644 --- a/Source/Engine/Content/Loading/ContentLoadingManager.cpp +++ b/Source/Engine/Content/Loading/ContentLoadingManager.cpp @@ -55,8 +55,7 @@ LoadingThread::~LoadingThread() // Check if has thread attached if (_thread != nullptr) { - if (_thread->IsRunning()) - _thread->Kill(true); + _thread->Kill(true); Delete(_thread); } } diff --git a/Source/Engine/Platform/Base/ThreadBase.cpp b/Source/Engine/Platform/Base/ThreadBase.cpp index 38e99ceb7..5739dbf3e 100644 --- a/Source/Engine/Platform/Base/ThreadBase.cpp +++ b/Source/Engine/Platform/Base/ThreadBase.cpp @@ -43,7 +43,10 @@ void ThreadBase::SetPriority(ThreadPriority priority) void ThreadBase::Kill(bool waitForJoin) { if (!_isRunning) + { + ClearHandleInternal(); return; + } ASSERT(GetID()); const auto thread = static_cast(this); @@ -105,7 +108,6 @@ int32 ThreadBase::Run() _callAfterWork = false; _runnable->AfterWork(false); } - ClearHandleInternal(); _isRunning = false; ThreadExiting(thread, exitCode); ThreadRegistry::Remove(thread); diff --git a/Source/Engine/Platform/Unix/UnixThread.cpp b/Source/Engine/Platform/Unix/UnixThread.cpp index 4fa532a4b..f662bdba8 100644 --- a/Source/Engine/Platform/Unix/UnixThread.cpp +++ b/Source/Engine/Platform/Unix/UnixThread.cpp @@ -53,6 +53,7 @@ UnixThread* UnixThread::Setup(UnixThread* thread, uint32 stackSize) void UnixThread::Join() { pthread_join(_thread, nullptr); + ClearHandleInternal(); } void UnixThread::ClearHandleInternal() diff --git a/Source/Engine/Platform/Win32/Win32Thread.cpp b/Source/Engine/Platform/Win32/Win32Thread.cpp index 5ede5cfc6..0cd8faaf2 100644 --- a/Source/Engine/Platform/Win32/Win32Thread.cpp +++ b/Source/Engine/Platform/Win32/Win32Thread.cpp @@ -113,6 +113,7 @@ unsigned long Win32Thread::ThreadProc(void* pThis) void Win32Thread::Join() { WaitForSingleObject((HANDLE)_thread, INFINITE); + ClearHandleInternal(); } void Win32Thread::ClearHandleInternal() diff --git a/Source/Engine/Threading/JobSystem.cpp b/Source/Engine/Threading/JobSystem.cpp index 1d4e9762a..396294e56 100644 --- a/Source/Engine/Threading/JobSystem.cpp +++ b/Source/Engine/Threading/JobSystem.cpp @@ -149,8 +149,7 @@ void JobSystemService::Dispose() { if (Threads[i]) { - if (Threads[i]->IsRunning()) - Threads[i]->Kill(true); + Threads[i]->Kill(true); Delete(Threads[i]); Threads[i] = nullptr; } diff --git a/Source/Engine/Threading/ThreadPool.cpp b/Source/Engine/Threading/ThreadPool.cpp index 2dd6176d0..b7db81ffa 100644 --- a/Source/Engine/Threading/ThreadPool.cpp +++ b/Source/Engine/Threading/ThreadPool.cpp @@ -98,10 +98,7 @@ void ThreadPoolService::Dispose() // Delete threads for (int32 i = 0; i < ThreadPoolImpl::Threads.Count(); i++) { - if (ThreadPoolImpl::Threads[i]->IsRunning()) - { - ThreadPoolImpl::Threads[i]->Kill(true); - } + ThreadPoolImpl::Threads[i]->Kill(true); } ThreadPoolImpl::Threads.ClearDelete(); }