From 3fb1f45301880a3aa901b752af1ecc912efe5915 Mon Sep 17 00:00:00 2001 From: Sebastien Marie Date: Thu, 10 Sep 2020 12:19:18 +0200 Subject: Remove Thread::kill() and related unittest (#10317) Closes: #6065 --- src/threading/thread.cpp | 54 +++++++++++++++++------------------------ src/threading/thread.h | 8 ------ src/unittest/test_threading.cpp | 25 ------------------- 3 files changed, 22 insertions(+), 65 deletions(-) diff --git a/src/threading/thread.cpp b/src/threading/thread.cpp index e19e6ae60..5cfc60995 100644 --- a/src/threading/thread.cpp +++ b/src/threading/thread.cpp @@ -73,7 +73,28 @@ Thread::Thread(const std::string &name) : Thread::~Thread() { - kill(); + // kill the thread if running + if (!m_running) { + wait(); + } else { + + m_running = false; + +#if defined(_WIN32) + // See https://msdn.microsoft.com/en-us/library/hh920601.aspx#thread__native_handle_method + TerminateThread((HANDLE) m_thread_obj->native_handle(), 0); + CloseHandle((HANDLE) m_thread_obj->native_handle()); +#else + // We need to pthread_kill instead on Android since NDKv5's pthread + // implementation is incomplete. +# ifdef __ANDROID__ + pthread_kill(getThreadHandle(), SIGKILL); +# else + pthread_cancel(getThreadHandle()); +# endif + wait(); +#endif + } // Make sure start finished mutex is unlocked before it's destroyed if (m_start_finished_mutex.try_lock()) @@ -138,37 +159,6 @@ bool Thread::wait() } -bool Thread::kill() -{ - if (!m_running) { - wait(); - return false; - } - - m_running = false; - -#if defined(_WIN32) - // See https://msdn.microsoft.com/en-us/library/hh920601.aspx#thread__native_handle_method - TerminateThread((HANDLE) m_thread_obj->native_handle(), 0); - CloseHandle((HANDLE) m_thread_obj->native_handle()); -#else - // We need to pthread_kill instead on Android since NDKv5's pthread - // implementation is incomplete. -# ifdef __ANDROID__ - pthread_kill(getThreadHandle(), SIGKILL); -# else - pthread_cancel(getThreadHandle()); -# endif - wait(); -#endif - - m_retval = nullptr; - m_joinable = false; - m_request_stop = false; - - return true; -} - bool Thread::getReturnValue(void **ret) { diff --git a/src/threading/thread.h b/src/threading/thread.h index 3946335f5..45fb171da 100644 --- a/src/threading/thread.h +++ b/src/threading/thread.h @@ -74,14 +74,6 @@ public: */ bool stop(); - /* - * Immediately terminates the thread. - * This should be used with extreme caution, as the thread will not have - * any opportunity to release resources it may be holding (such as memory - * or locks). - */ - bool kill(); - /* * Waits for thread to finish. * Note: This does not stop a thread, you have to do this on your own. diff --git a/src/unittest/test_threading.cpp b/src/unittest/test_threading.cpp index 8d4d814fd..65ef7c02d 100644 --- a/src/unittest/test_threading.cpp +++ b/src/unittest/test_threading.cpp @@ -31,7 +31,6 @@ public: void runTests(IGameDef *gamedef); void testStartStopWait(); - void testThreadKill(); void testAtomicSemaphoreThread(); }; @@ -40,7 +39,6 @@ static TestThreading g_test_instance; void TestThreading::runTests(IGameDef *gamedef) { TEST(testStartStopWait); - TEST(testThreadKill); TEST(testAtomicSemaphoreThread); } @@ -111,29 +109,6 @@ void TestThreading::testStartStopWait() } -void TestThreading::testThreadKill() -{ - SimpleTestThread *thread = new SimpleTestThread(300); - - UASSERT(thread->start() == true); - - // kill()ing is quite violent, so let's make sure our victim is sleeping - // before we do this... so we don't corrupt the rest of the program's state - sleep_ms(100); - UASSERT(thread->kill() == true); - - // The state of the thread object should be reset if all went well - UASSERT(thread->isRunning() == false); - UASSERT(thread->start() == true); - UASSERT(thread->stop() == true); - UASSERT(thread->wait() == true); - - // kill() after already waiting should fail. - UASSERT(thread->kill() == false); - - delete thread; -} - class AtomicTestThread : public Thread { public: -- cgit v1.2.3