From 3fb1f45301880a3aa901b752af1ecc912efe5915 Mon Sep 17 00:00:00 2001
From: Sebastien Marie <semarie@users.noreply.github.com>
Date: Thu, 10 Sep 2020 12:19:18 +0200
Subject: [PATCH] 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: