Remove Thread::kill() and related unittest (#10317)

Closes: #6065

diff --git a/src/threading/thread.cpp b/src/threading/thread.cpp
index e19e6ae60cc24ff965f1197df7e84c514bec14f0..5cfc609957eb5ed999afddb932c634a4f3f39be4 100644 (file)
--- 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 3946335f5f63b5e7fe042a7bc441185470632295..45fb171daf3b3554885b1a40bb0eccc22871614c 100644 (file)
--- a/src/threading/thread.h
+++ b/src/threading/thread.h
@@ -74,14 +74,6 @@ class Thread {
         */
        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 8d4d814fd3ede8f24fd61b680c007a5b7c71981e..65ef7c02d77d96b02a0c6c5897a9977f63003913 100644 (file)
--- a/src/unittest/test_threading.cpp
+++ b/src/unittest/test_threading.cpp
@@ -31,7 +31,6 @@ class TestThreading : public TestBase {
        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: