Fix some threading things and add additional thread unittests

- Fix thread name reset on start()
- Fully reset thread state on kill()
- Add unittests to check for correct object states under various circumstances

2 files changed, 33 insertions, 48 deletions

diff --git a/src/threading/thread.cpp b/src/threading/thread.cpp
index cca7e636e..0996a9e34 100644
--- a/src/threading/thread.cpp
+++ b/src/threading/thread.cpp
@@ -88,16 +88,13 @@ DEALINGS IN THE SOFTWARE.
 Thread::Thread(const std::string &name) :
 	m_name(name),
 	m_retval(NULL),
+	m_joinable(false),
 	m_request_stop(false),
 	m_running(false)
 {
 #ifdef _AIX
 	m_kernel_thread_id = -1;
 #endif
-
-#if USE_CPP11_THREADS
-	m_thread_obj = NULL;
-#endif
 }
 
 
@@ -109,12 +106,12 @@ Thread::~Thread()
 
 bool Thread::start()
 {
-	MutexAutoLock lock(m_continue_mutex);
+	MutexAutoLock lock(m_mutex);
 
 	if (m_running)
 		return false;
 
-	cleanup();
+	m_request_stop = false;
 
 #if USE_CPP11_THREADS
 
@@ -145,6 +142,8 @@ bool Thread::start()
 	while (!m_running)
 		sleep_ms(1);
 
+	m_joinable = true;
+
 	return true;
 }
 
@@ -156,21 +155,30 @@ bool Thread::stop()
 }
 
 
-void Thread::wait()
+bool Thread::wait()
 {
-	if (!m_running)
-		return;
+	MutexAutoLock lock(m_mutex);
+
+	if (!m_joinable)
+		return false;
 
 #if USE_CPP11_THREADS
 
 	m_thread_obj->join();
 
+	delete m_thread_obj;
+	m_thread_obj = NULL;
+
 #elif USE_WIN_THREADS
 
 	int ret = WaitForSingleObject(m_thread_handle, INFINITE);
 	assert(ret == WAIT_OBJECT_0);
 	UNUSED(ret);
 
+	CloseHandle(m_thread_handle);
+	m_thread_handle = NULL;
+	m_thread_id = -1;
+
 #elif USE_POSIX_THREADS
 
 	int ret = pthread_join(m_thread_handle, NULL);
@@ -180,8 +188,8 @@ void Thread::wait()
 #endif
 
 	assert(m_running == false);
-
-	return;
+	m_joinable = false;
+	return true;
 }
 
 
@@ -192,10 +200,12 @@ bool Thread::kill()
 		return false;
 	}
 
+	m_running = false;
+
 #ifdef _WIN32
 	TerminateThread(m_thread_handle, 0);
+	CloseHandle(m_thread_handle);
 #else
-
 	// We need to pthread_kill instead on Android since NDKv5's pthread
 	// implementation is incomplete.
 # ifdef __ANDROID__
@@ -203,39 +213,14 @@ bool Thread::kill()
 # else
 	pthread_cancel(m_thread_handle);
 # endif
-
 	wait();
 #endif
 
-	cleanup();
-
-	return true;
-}
-
-
-void Thread::cleanup()
-{
-#if USE_CPP11_THREADS
-
-	delete m_thread_obj;
-	m_thread_obj = NULL;
-
-#elif USE_WIN_THREADS
-
-	CloseHandle(m_thread_handle);
-	m_thread_handle = NULL;
-	m_thread_id = -1;
-
-#elif USE_POSIX_THREADS
-
-	// Can't do any cleanup for pthreads
-
-#endif
-
-	m_name         = "";
 	m_retval       = NULL;
-	m_running      = false;
+	m_joinable     = false;
 	m_request_stop = false;
+
+	return true;
 }
 
 
@@ -256,11 +241,11 @@ bool Thread::isCurrentThread()
 
 
 #if USE_CPP11_THREADS || USE_POSIX_THREADS
-	void *(Thread::threadProc)(void *param)
+void *Thread::threadProc(void *param)
 #elif defined(_WIN32_WCE)
-	DWORD (Thread::threadProc)(LPVOID param)
+DWORD Thread::threadProc(LPVOID param)
 #elif defined(_WIN32)
-	DWORD WINAPI (Thread::threadProc)(LPVOID param)
+DWORD WINAPI Thread::threadProc(LPVOID param)
 #endif
 {
 	Thread *thr = (Thread *)param;
diff --git a/src/threading/thread.h b/src/threading/thread.h
index cd46856e9..3d85e0eb9 100644
--- a/src/threading/thread.h
+++ b/src/threading/thread.h
@@ -81,9 +81,10 @@ public:
 	/*
 	 * Waits for thread to finish.
 	 * Note:  This does not stop a thread, you have to do this on your own.
-	 * Returns immediately if the thread is not started.
+	 * Returns false immediately if the thread is not started or has been waited
+	 * on before.
 	 */
-	void wait();
+	bool wait();
 
 	/*
 	 * Returns true if the calling thread is this Thread object.
@@ -140,15 +141,14 @@ protected:
 
 private:
 	void *m_retval;
+	bool m_joinable;
 	Atomic<bool> m_request_stop;
 	Atomic<bool> m_running;
-	Mutex m_continue_mutex;
+	Mutex m_mutex;
 
 	threadid_t m_thread_id;
 	threadhandle_t m_thread_handle;
 
-	void cleanup();
-
 	static ThreadStartFunc threadProc;
 
 #ifdef _AIX