Refactor Thread class to improve readability and portability

- Fix some incompatibilities with obscure platforms (AIX and WinCE)
- Clean up Thread class interface
- Add m_ prefix to private member variables
- Simplify platform-dependent logic, reducing preprocessor
  conditional clauses and improving readibility
- Add Thread class documentation

src/emerge.cpp

@@ -219,7 +219,7 @@ void EmergeManager::initMapgens()
 Mapgen *EmergeManager::getCurrentMapgen()
 {
 	for (u32 i = 0; i != m_threads.size(); i++) {
-		if (m_threads[i]->isSameThread())
+		if (m_threads[i]->isCurrentThread())
 			return m_threads[i]->m_mapgen;
 	}
 
@@ -476,7 +476,7 @@ EmergeThread::EmergeThread(Server *server, int ethreadid) :
 	m_emerge(NULL),
 	m_mapgen(NULL)
 {
-	name = "Emerge-" + itos(ethreadid);
+	m_name = "Emerge-" + itos(ethreadid);
 }
 
 

src/threading/thread.cpp

@@ -26,23 +26,24 @@ DEALINGS IN THE SOFTWARE.
 #include "threading/thread.h"
 #include "threading/mutex_auto_lock.h"
 #include "log.h"
+#include "porting.h"
 
-#if __cplusplus >= 201103L
-	#include <chrono>
-#else
 #define UNUSED(expr) do { (void)(expr); } while (0)
-# ifdef _WIN32
+
+#if USE_CPP11_THREADS
+	#include <chrono>
+	#include <system_error>
+#elif USE_WIN_THREADS
 	#ifndef _WIN32_WCE
 		#include <process.h>
 	#endif
-# else
+#elif USE_POSIX_THREADS
-	#include <ctime>
+	#include <time.h>
-	#include <cassert>
+	#include <assert.h>
-	#include <cstdlib>
+	#include <stdlib.h>
+	#include <unistd.h>
 	#include <sys/time.h>
 
-	// For getNumberOfProcessors
-	#include <unistd.h>
 	#if defined(__FreeBSD__) || defined(__APPLE__)
 		#include <sys/types.h>
 		#include <sys/sysctl.h>
@@ -50,10 +51,9 @@ DEALINGS IN THE SOFTWARE.
 		#include <sys/sysinfo.h>
 	#endif
 #endif
-#endif
 
 
-// For setName
+// for setName
 #if defined(linux) || defined(__linux)
 	#include <sys/prctl.h>
 #elif defined(__FreeBSD__) || defined(__OpenBSD__)
@@ -67,7 +67,7 @@ DEALINGS IN THE SOFTWARE.
 	};
 #endif
 
-// For bindToProcessor
+// for bindToProcessor
 #if __FreeBSD_version >= 702106
 	typedef cpuset_t cpu_set_t;
 #elif defined(__linux) || defined(linux)
@@ -78,6 +78,7 @@ DEALINGS IN THE SOFTWARE.
 	#include <sys/procset.h>
 #elif defined(_AIX)
 	#include <sys/processor.h>
+	#include <sys/thread.h>
 #elif defined(__APPLE__)
 	#include <mach/mach_init.h>
 	#include <mach/thread_act.h>
@@ -85,188 +86,246 @@ DEALINGS IN THE SOFTWARE.
 
 
 Thread::Thread(const std::string &name) :
-	name(name),
+	m_name(name),
-	retval(NULL),
+	m_retval(NULL),
-	request_stop(false),
+	m_request_stop(false),
-	running(false)
+	m_running(false)
-#if __cplusplus >= 201103L
-	, thread(NULL)
-#elif !defined(_WIN32)
-	, started(false)
-#endif
-{}
-
-
-void Thread::wait()
 {
-#if __cplusplus >= 201103L
+#ifdef _AIX
-	if (!thread || !thread->joinable())
+	m_kernel_thread_id = -1;
-		return;
-	thread->join();
-#elif defined(_WIN32)
-	if (!running)
-		return;
-	WaitForSingleObject(thread, INFINITE);
-#else // pthread
-	void *status;
-	if (!started)
-		return;
-	int ret = pthread_join(thread, &status);
-	assert(!ret);
-	UNUSED(ret);
-	started = false;
 #endif
+
+#if USE_CPP11_THREADS
+	m_thread_obj = NULL;
+#endif
+}
+
+
+Thread::~Thread()
+{
+	kill();
 }
 
 
 bool Thread::start()
 {
-	if (running)
+	MutexAutoLock lock(m_continue_mutex);
+
+	if (m_running)
 		return false;
-	request_stop = false;
 
-#if __cplusplus >= 201103L
+	cleanup();
-	MutexAutoLock l(continue_mutex);
+
-	thread = new std::thread(theThread, this);
+#if USE_CPP11_THREADS
-#elif defined(_WIN32)
+
-	MutexAutoLock l(continue_mutex);
+	try {
-# ifdef _WIN32_WCE
+		m_thread_obj    = new std::thread(threadProc, this);
-	thread = CreateThread(NULL, 0, theThread, this, 0, &thread_id);
+		m_thread_id     = m_thread->get_id();
-# else
+		m_thread_handle = m_thread->native_handle();
-	thread = (HANDLE)_beginthreadex(NULL, 0, theThread, this, 0, &thread_id);
+	} except (const std::system_error &e) {
-# endif
-	if (!thread)
 		return false;
-#else
+	}
-	int status;
 
-	MutexAutoLock l(continue_mutex);
+#elif USE_WIN_THREADS
 
-	status = pthread_create(&thread, NULL, theThread, this);
+	m_thread_handle = CreateThread(NULL, 0, threadProc, this, 0, &m_thread_id);
+	if (!m_thread_handle)
+		return false;
 
+#elif USE_POSIX_THREADS
+
+	int status = pthread_create(&m_thread_handle, NULL, threadProc, this);
 	if (status)
 		return false;
+
+	m_thread_id = m_thread_handle;
+
 #endif
 
-#if __cplusplus < 201103L
+	while (!m_running)
-	// Wait until running
+		sleep_ms(1);
-	while (!running) {
+
-# ifdef _WIN32
-		Sleep(1);
-	}
-# else
-		struct timespec req, rem;
-		req.tv_sec = 0;
-		req.tv_nsec = 1000000;
-		nanosleep(&req, &rem);
-	}
-	started = true;
-# endif
-#endif
 	return true;
 }
 
 
+bool Thread::stop()
+{
+	m_request_stop = true;
+	return true;
+}
+
+
+void Thread::wait()
+{
+	if (!m_running)
+		return;
+
+#if USE_CPP11_THREADS
+
+	m_thread_obj->join();
+
+#elif USE_WIN_THREADS
+
+	int ret == WaitForSingleObject(m_thread_handle, INFINITE);
+	assert(ret == WAIT_OBJECT_0);
+	UNUSED(ret);
+
+#elif USE_POSIX_THREADS
+
+	int ret = pthread_join(m_thread_handle, NULL);
+	assert(ret == 0);
+	UNUSED(ret);
+
+#endif
+
+	assert(m_running == false);
+
+	return;
+}
+
+
 bool Thread::kill()
 {
-#ifdef _WIN32
+	if (!m_running) {
-	if (!running)
-		return false;
-	TerminateThread(getThreadHandle(), 0);
-	CloseHandle(getThreadHandle());
-#else
-	if (!running) {
 		wait();
 		return false;
 	}
-# ifdef __ANDROID__
+
-	pthread_kill(getThreadHandle(), SIGKILL);
+#ifdef _WIN32
+	TerminateThread(m_thread_handle, 0);
 #else
-	pthread_cancel(getThreadHandle());
+
+	// We need to pthread_kill instead on Android since NDKv5's pthread
+	// implementation is incomplete.
+# ifdef __ANDROID__
+	pthread_kill(m_thread_handle, SIGKILL);
+# else
+	pthread_cancel(m_thread_handle);
 # endif
+
 	wait();
 #endif
-#if __cplusplus >= 201103L
+
-	delete thread;
+	cleanup();
-#endif
+
-	running = false;
 	return true;
 }
 
 
-bool Thread::isSameThread()
+void Thread::cleanup()
 {
-#if __cplusplus >= 201103L
+#if USE_CPP11_THREADS
-	return thread->get_id() == std::this_thread::get_id();
+
-#elif defined(_WIN32)
+	delete m_thread_obj;
-	return GetCurrentThreadId() == thread_id;
+	m_thread_obj = NULL;
-#else
+
-	return pthread_equal(pthread_self(), thread);
+#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_request_stop = false;
 }
 
 
-#if __cplusplus >= 201103L
+bool Thread::getReturnValue(void **ret)
-void Thread::theThread(Thread *th)
+{
+	if (m_running)
+		return false;
+
+	*ret = m_retval;
+	return true;
+}
+
+
+bool Thread::isCurrentThread()
+{
+	return thr_is_current_thread(m_thread_id);
+}
+
+
+#if USE_CPP11_THREADS || USE_POSIX_THREADS
+	void *(Thread::threadProc)(void *param)
 #elif defined(_WIN32_WCE)
-DWORD WINAPI Thread::theThread(void *param)
+	DWORD (Thread::threadProc)(LPVOID param)
 #elif defined(_WIN32)
-UINT __stdcall Thread::theThread(void *param)
+	DWORD WINAPI (Thread::threadProc)(LPVOID param)
-#else
-void *Thread::theThread(void *param)
 #endif
 {
-#if __cplusplus < 201103L
+	Thread *thr = (Thread *)param;
-	Thread *th = static_cast<Thread *>(param);
+
+#ifdef _AIX
+	m_kernel_thread_id = thread_self();
 #endif
-	th->running = true;
 
-	th->setName();
+	thr->setName(thr->m_name);
-	g_logger.registerThread(th->name);
 
-	th->retval = th->run();
+	g_logger.registerThread(thr->m_name);
+	thr->m_running = true;
 
+	thr->m_retval = thr->run();
+
+	thr->m_running = false;
 	g_logger.deregisterThread();
 
-	th->running = false;
-#if __cplusplus < 201103L
-# ifdef _WIN32
-	CloseHandle(th->thread);
-# endif
 	return NULL;
-#endif
 }
 
 
 void Thread::setName(const std::string &name)
 {
 #if defined(linux) || defined(__linux)
-	/* It would be cleaner to do this with pthread_setname_np,
+
-	 * which was added to glibc in version 2.12, but some major
+	// It would be cleaner to do this with pthread_setname_np,
-	 * distributions are still runing 2.11 and previous versions.
+	// which was added to glibc in version 2.12, but some major
-	 */
+	// distributions are still runing 2.11 and previous versions.
 	prctl(PR_SET_NAME, name.c_str());
+
 #elif defined(__FreeBSD__) || defined(__OpenBSD__)
+
 	pthread_set_name_np(pthread_self(), name.c_str());
+
 #elif defined(__NetBSD__)
+
 	pthread_setname_np(pthread_self(), name.c_str());
+
 #elif defined(__APPLE__)
+
 	pthread_setname_np(name.c_str());
+
 #elif defined(_MSC_VER)
+
 	// Windows itself doesn't support thread names,
 	// but the MSVC debugger does...
 	THREADNAME_INFO info;
+
 	info.dwType = 0x1000;
 	info.szName = name.c_str();
 	info.dwThreadID = -1;
 	info.dwFlags = 0;
+
 	__try {
-		RaiseException(0x406D1388, 0, sizeof(info) / sizeof(DWORD), (ULONG_PTR *)&info);
+		RaiseException(0x406D1388, 0,
+			sizeof(info) / sizeof(DWORD), (ULONG_PTR *)&info);
 	} __except (EXCEPTION_CONTINUE_EXECUTION) {
 	}
+
 #elif defined(_WIN32) || defined(__GNU__)
+
 	// These platforms are known to not support thread names.
 	// Silently ignore the request.
+
 #else
 	#warning "Unrecognized platform, thread names will not be available."
 #endif
@@ -276,59 +335,96 @@ void Thread::setName(const std::string &name)
 unsigned int Thread::getNumberOfProcessors()
 {
 #if __cplusplus >= 201103L
+
 	return std::thread::hardware_concurrency();
+
 #elif defined(_SC_NPROCESSORS_ONLN)
+
 	return sysconf(_SC_NPROCESSORS_ONLN);
-#elif defined(__FreeBSD__) || defined(__APPLE__)
+
-	unsigned int len, count;
+#elif defined(__FreeBSD__) || defined(__NetBSD__) || \
-	len = sizeof(count);
+	defined(__DragonFly__) || defined(__APPLE__)
-	return sysctlbyname("hw.ncpu", &count, &len, NULL, 0);
+
+	unsigned int num_cpus = 1;
+	size_t len = sizeof(num_cpus);
+
+	int mib[2];
+	mib[0] = CTL_HW;
+	mib[1] = HW_NCPU;
+
+	sysctl(mib, 2, &num_cpus, &len, NULL, 0);
+	return num_cpus;
+
 #elif defined(_GNU_SOURCE)
+
 	return get_nprocs();
+
 #elif defined(_WIN32)
+
 	SYSTEM_INFO sysinfo;
 	GetSystemInfo(&sysinfo);
 	return sysinfo.dwNumberOfProcessors;
+
 #elif defined(PTW32_VERSION) || defined(__hpux)
+
 	return pthread_num_processors_np();
+
 #else
+
 	return 1;
+
 #endif
 }
 
 
-bool Thread::bindToProcessor(unsigned int num)
+bool Thread::bindToProcessor(unsigned int proc_number)
 {
 #if defined(__ANDROID__)
+
 	return false;
+
 #elif defined(_WIN32)
-	return SetThreadAffinityMask(getThreadHandle(), 1 << num);
+
+	return SetThreadAffinityMask(m_thread_handle, 1 << proc_number);
+
 #elif __FreeBSD_version >= 702106 || defined(__linux) || defined(linux)
+
 	cpu_set_t cpuset;
+
 	CPU_ZERO(&cpuset);
-	CPU_SET(num, &cpuset);
+	CPU_SET(proc_number, &cpuset);
-	return pthread_setaffinity_np(getThreadHandle(), sizeof(cpuset),
+
-		&cpuset) == 0;
+	return pthread_setaffinity_np(m_thread_handle, sizeof(cpuset), &cpuset) == 0;
+
 #elif defined(__sun) || defined(sun)
-	return processor_bind(P_LWPID, MAKE_LWPID_PTHREAD(getThreadHandle()),
+
-			num, NULL) == 0
+	return processor_bind(P_LWPID, P_MYID, proc_number, NULL) == 0
+
 #elif defined(_AIX)
-	return bindprocessor(BINDTHREAD, (tid_t) getThreadHandle(), pnumber) == 0;
+
+	return bindprocessor(BINDTHREAD, m_kernel_thread_id, proc_number) == 0;
+
 #elif defined(__hpux) || defined(hpux)
+
 	pthread_spu_t answer;
 
 	return pthread_processor_bind_np(PTHREAD_BIND_ADVISORY_NP,
-			&answer, num, getThreadHandle()) == 0;
+			&answer, proc_number, m_thread_handle) == 0;
+
 #elif defined(__APPLE__)
+
 	struct thread_affinity_policy tapol;
 
-	thread_port_t threadport = pthread_mach_thread_np(getThreadHandle());
+	thread_port_t threadport = pthread_mach_thread_np(m_thread_handle);
-	tapol.affinity_tag = num + 1;
+	tapol.affinity_tag = proc_number + 1;
 	return thread_policy_set(threadport, THREAD_AFFINITY_POLICY,
 			(thread_policy_t)&tapol,
 			THREAD_AFFINITY_POLICY_COUNT) == KERN_SUCCESS;
+
 #else
+
 	return false;
+
 #endif
 }
 
@@ -336,19 +432,23 @@ bool Thread::bindToProcessor(unsigned int num)
 bool Thread::setPriority(int prio)
 {
 #if defined(_WIN32)
-	return SetThreadPriority(getThreadHandle(), prio);
+
+	return SetThreadPriority(m_thread_handle, prio);
+
 #else
+
 	struct sched_param sparam;
 	int policy;
 
-	if (pthread_getschedparam(getThreadHandle(), &policy, &sparam) != 0)
+	if (pthread_getschedparam(m_thread_handle, &policy, &sparam) != 0)
 		return false;
 
 	int min = sched_get_priority_min(policy);
 	int max = sched_get_priority_max(policy);
 
 	sparam.sched_priority = min + prio * (max - min) / THREAD_PRIORITY_HIGHEST;
-	return pthread_setschedparam(getThreadHandle(), policy, &sparam) == 0;
+	return pthread_setschedparam(m_thread_handle, policy, &sparam) == 0;
+
 #endif
 }
 

src/threading/thread.h

@@ -28,91 +28,136 @@ DEALINGS IN THE SOFTWARE.
 
 #include "threading/atomic.h"
 #include "threading/mutex.h"
+#include "threads.h"
 
 #include <string>
-#if __cplusplus >= 201103L
+#if USE_CPP11_THREADS
 	#include <thread>
 #endif
 
-#ifndef _WIN32
+/*
-enum {
+ * On platforms using pthreads, these five priority classes correlate to
-	THREAD_PRIORITY_LOWEST,
+ * even divisions between the minimum and maximum reported thread priority.
-	THREAD_PRIORITY_BELOW_NORMAL,
+ */
-	THREAD_PRIORITY_NORMAL,
+#if !defined(_WIN32)
-	THREAD_PRIORITY_ABOVE_NORMAL,
+	#define THREAD_PRIORITY_LOWEST       0
-	THREAD_PRIORITY_HIGHEST,
+	#define THREAD_PRIORITY_BELOW_NORMAL 1
-};
+	#define THREAD_PRIORITY_NORMAL       2
+	#define THREAD_PRIORITY_ABOVE_NORMAL 3
+	#define THREAD_PRIORITY_HIGHEST      4
 #endif
 
 
-class Thread
+class Thread {
-{
 public:
-	Thread(const std::string &name="Unnamed");
+	Thread(const std::string &name="");
-	virtual ~Thread() { kill(); }
+	virtual ~Thread();
-
-	bool start();
-	inline void stop() { request_stop = true; }
-	bool kill();
-
-	inline bool isRunning() { return running; }
-	inline bool stopRequested() { return request_stop; }
-	void *getReturnValue() { return running ? NULL : retval; }
-	bool isSameThread();
-
-	static unsigned int getNumberOfProcessors();
-	bool bindToProcessor(unsigned int);
-	bool setPriority(int);
 
 	/*
-	 * Wait for thread to finish.
+	 * Begins execution of a new thread at the pure virtual method Thread::run().
-	 * Note: this does not stop a thread, you have to do this on your own.
+	 * Execution of the thread is guaranteed to have started after this function
+	 * returns.
+	 */
+	bool start();
+
+	/*
+	 * Requests that the thread exit gracefully.
+	 * Returns immediately; thread execution is guaranteed to be complete after
+	 * a subsequent call to Thread::wait.
+	 */
+	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.
 	 * Returns immediately if the thread is not started.
 	 */
 	void wait();
 
+	/*
+	 * Returns true if the calling thread is this Thread object.
+	 */
+	bool isCurrentThread();
+
+	inline bool isRunning() { return m_running; }
+	inline bool stopRequested() { return m_request_stop; }
+	inline threadid_t getThreadId() { return m_thread_id; }
+	inline threadhandle_t getThreadHandle() { return m_thread_handle; }
+
+	/*
+	 * Gets the thread return value.
+	 * Returns true if the thread has exited and the return value was available,
+	 * or false if the thread has yet to finish.
+	 */
+	bool getReturnValue(void **ret);
+
+	/*
+	 * Binds (if possible, otherwise sets the affinity of) the thread to the
+	 * specific processor specified by proc_number.
+	 */
+	bool bindToProcessor(unsigned int proc_number);
+
+	/*
+	 * Sets the thread priority to the specified priority.
+	 *
+	 * prio can be one of: THREAD_PRIORITY_LOWEST, THREAD_PRIORITY_BELOW_NORMAL,
+	 * THREAD_PRIORITY_NORMAL, THREAD_PRIORITY_ABOVE_NORMAL, THREAD_PRIORITY_HIGHEST.
+	 * On Windows, any of the other priorites as defined by SetThreadPriority
+	 * are supported as well.
+	 *
+	 * Note that it may be necessary to first set the threading policy or
+	 * scheduling algorithm to one that supports thread priorities if not
+	 * supported by default, otherwise this call will have no effect.
+	 */
+	bool setPriority(int prio);
+
+	/*
+	 * Sets the currently executing thread's name to where supported; useful
+	 * for debugging.
+	 */
 	static void setName(const std::string &name);
 
+	/*
+	 * Returns the number of processors/cores configured and active on this machine.
+	 */
+	static unsigned int getNumberOfProcessors();
+
 protected:
-	std::string name;
+	std::string m_name;
 
 	virtual void *run() = 0;
 
 private:
-	void setName() { setName(name); }
+	void *m_retval;
+	Atomic<bool> m_request_stop;
+	Atomic<bool> m_running;
+	Mutex m_continue_mutex;
 
-	void *retval;
+	threadid_t m_thread_id;
-	Atomic<bool> request_stop;
+	threadhandle_t m_thread_handle;
-	Atomic<bool> running;
-	Mutex continue_mutex;
 
-#if __cplusplus >= 201103L
+	void cleanup();
-	static void theThread(Thread *th);
 
-	std::thread *thread;
+	static ThreadStartFunc threadProc;
-	std::thread::native_handle_type getThreadHandle() const
+
-		{ return thread->native_handle(); }
+#ifdef _AIX
-#else
+	// For AIX, there does not exist any mapping from pthread_t to tid_t
-# if defined(WIN32) || defined(_WIN32_WCE)
+	// available to us, so we maintain one ourselves.  This is set on thread start.
-#  ifdef _WIN32_WCE
+	tid_t m_kernel_thread_id;
-	DWORD thread_id;
-	static DWORD WINAPI theThread(void *param);
-#  else
-	UINT thread_id;
-	static UINT __stdcall theThread(void *param);
 #endif
 
-	HANDLE thread;
+#if USE_CPP11_THREADS
-	HANDLE getThreadHandle() const { return thread; }
+	std::thread *m_thread_obj;
-# else // pthread
-	static void *theThread(void *param);
-
-	pthread_t thread;
-	pthread_t getThreadHandle() const { return thread; }
-
-	Atomic<bool> started;
-# endif
 #endif
+
 };
 
 #endif