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
This commit is contained in:
kwolekr 2015-10-17 22:42:48 -04:00
parent 59fa117d13
commit 964be640cb
4 changed files with 137 additions and 55 deletions

@ -88,16 +88,13 @@ DEALINGS IN THE SOFTWARE.
Thread::Thread(const std::string &name) : Thread::Thread(const std::string &name) :
m_name(name), m_name(name),
m_retval(NULL), m_retval(NULL),
m_joinable(false),
m_request_stop(false), m_request_stop(false),
m_running(false) m_running(false)
{ {
#ifdef _AIX #ifdef _AIX
m_kernel_thread_id = -1; m_kernel_thread_id = -1;
#endif #endif
#if USE_CPP11_THREADS
m_thread_obj = NULL;
#endif
} }
@ -109,12 +106,12 @@ Thread::~Thread()
bool Thread::start() bool Thread::start()
{ {
MutexAutoLock lock(m_continue_mutex); MutexAutoLock lock(m_mutex);
if (m_running) if (m_running)
return false; return false;
cleanup(); m_request_stop = false;
#if USE_CPP11_THREADS #if USE_CPP11_THREADS
@ -145,6 +142,8 @@ bool Thread::start()
while (!m_running) while (!m_running)
sleep_ms(1); sleep_ms(1);
m_joinable = true;
return true; return true;
} }
@ -156,21 +155,30 @@ bool Thread::stop()
} }
void Thread::wait() bool Thread::wait()
{ {
if (!m_running) MutexAutoLock lock(m_mutex);
return;
if (!m_joinable)
return false;
#if USE_CPP11_THREADS #if USE_CPP11_THREADS
m_thread_obj->join(); m_thread_obj->join();
delete m_thread_obj;
m_thread_obj = NULL;
#elif USE_WIN_THREADS #elif USE_WIN_THREADS
int ret = WaitForSingleObject(m_thread_handle, INFINITE); int ret = WaitForSingleObject(m_thread_handle, INFINITE);
assert(ret == WAIT_OBJECT_0); assert(ret == WAIT_OBJECT_0);
UNUSED(ret); UNUSED(ret);
CloseHandle(m_thread_handle);
m_thread_handle = NULL;
m_thread_id = -1;
#elif USE_POSIX_THREADS #elif USE_POSIX_THREADS
int ret = pthread_join(m_thread_handle, NULL); int ret = pthread_join(m_thread_handle, NULL);
@ -180,8 +188,8 @@ void Thread::wait()
#endif #endif
assert(m_running == false); assert(m_running == false);
m_joinable = false;
return; return true;
} }
@ -192,10 +200,12 @@ bool Thread::kill()
return false; return false;
} }
m_running = false;
#ifdef _WIN32 #ifdef _WIN32
TerminateThread(m_thread_handle, 0); TerminateThread(m_thread_handle, 0);
CloseHandle(m_thread_handle);
#else #else
// We need to pthread_kill instead on Android since NDKv5's pthread // We need to pthread_kill instead on Android since NDKv5's pthread
// implementation is incomplete. // implementation is incomplete.
# ifdef __ANDROID__ # ifdef __ANDROID__
@ -203,42 +213,17 @@ bool Thread::kill()
# else # else
pthread_cancel(m_thread_handle); pthread_cancel(m_thread_handle);
# endif # endif
wait(); wait();
#endif #endif
cleanup(); m_retval = NULL;
m_joinable = false;
m_request_stop = false;
return true; 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_request_stop = false;
}
bool Thread::getReturnValue(void **ret) bool Thread::getReturnValue(void **ret)
{ {
if (m_running) if (m_running)
@ -256,11 +241,11 @@ bool Thread::isCurrentThread()
#if USE_CPP11_THREADS || USE_POSIX_THREADS #if USE_CPP11_THREADS || USE_POSIX_THREADS
void *(Thread::threadProc)(void *param) void *Thread::threadProc(void *param)
#elif defined(_WIN32_WCE) #elif defined(_WIN32_WCE)
DWORD (Thread::threadProc)(LPVOID param) DWORD Thread::threadProc(LPVOID param)
#elif defined(_WIN32) #elif defined(_WIN32)
DWORD WINAPI (Thread::threadProc)(LPVOID param) DWORD WINAPI Thread::threadProc(LPVOID param)
#endif #endif
{ {
Thread *thr = (Thread *)param; Thread *thr = (Thread *)param;

@ -81,9 +81,10 @@ public:
/* /*
* Waits for thread to finish. * Waits for thread to finish.
* Note: This does not stop a thread, you have to do this on your own. * 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. * Returns true if the calling thread is this Thread object.
@ -140,15 +141,14 @@ protected:
private: private:
void *m_retval; void *m_retval;
bool m_joinable;
Atomic<bool> m_request_stop; Atomic<bool> m_request_stop;
Atomic<bool> m_running; Atomic<bool> m_running;
Mutex m_continue_mutex; Mutex m_mutex;
threadid_t m_thread_id; threadid_t m_thread_id;
threadhandle_t m_thread_handle; threadhandle_t m_thread_handle;
void cleanup();
static ThreadStartFunc threadProc; static ThreadStartFunc threadProc;
#ifdef _AIX #ifdef _AIX

@ -58,11 +58,11 @@ with this program; if not, write to the Free Software Foundation, Inc.,
// ThreadStartFunc // ThreadStartFunc
// //
#if USE_CPP11_THREADS || USE_POSIX_THREADS #if USE_CPP11_THREADS || USE_POSIX_THREADS
typedef void *(ThreadStartFunc)(void *param); typedef void *ThreadStartFunc(void *param);
#elif defined(_WIN32_WCE) #elif defined(_WIN32_WCE)
typedef DWORD (ThreadStartFunc)(LPVOID param); typedef DWORD ThreadStartFunc(LPVOID param);
#elif defined(_WIN32) #elif defined(_WIN32)
typedef DWORD WINAPI (ThreadStartFunc)(LPVOID param); typedef DWORD WINAPI ThreadStartFunc(LPVOID param);
#endif #endif

@ -28,26 +28,122 @@ class TestThreading : public TestBase {
public: public:
TestThreading() { TestManager::registerTestModule(this); } TestThreading() { TestManager::registerTestModule(this); }
const char *getName() { return "TestThreading"; } const char *getName() { return "TestThreading"; }
void runTests(IGameDef *); void runTests(IGameDef *gamedef);
void testStartStopWait();
void testThreadKill();
void testAtomicSemaphoreThread(); void testAtomicSemaphoreThread();
}; };
static TestThreading g_test_instance; static TestThreading g_test_instance;
void TestThreading::runTests(IGameDef *) void TestThreading::runTests(IGameDef *gamedef)
{ {
TEST(testStartStopWait);
TEST(testThreadKill);
TEST(testAtomicSemaphoreThread); TEST(testAtomicSemaphoreThread);
} }
class SimpleTestThread : public Thread {
public:
SimpleTestThread(unsigned int interval) :
Thread("SimpleTest"),
m_interval(interval)
{
}
class AtomicTestThread : public Thread private:
void *run()
{
void *retval = this;
if (isCurrentThread() == false)
retval = (void *)0xBAD;
while (!stopRequested())
sleep_ms(m_interval);
return retval;
}
unsigned int m_interval;
};
void TestThreading::testStartStopWait()
{ {
void *thread_retval;
SimpleTestThread *thread = new SimpleTestThread(25);
// Try this a couple times, since a Thread should be reusable after waiting
for (size_t i = 0; i != 5; i++) {
// Can't wait() on a joined, stopped thread
UASSERT(thread->wait() == false);
// start() should work the first time, but not the second.
UASSERT(thread->start() == true);
UASSERT(thread->start() == false);
UASSERT(thread->isRunning() == true);
UASSERT(thread->isCurrentThread() == false);
// Let it loop a few times...
sleep_ms(70);
// It's still running, so the return value shouldn't be available to us.
UASSERT(thread->getReturnValue(&thread_retval) == false);
// stop() should always succeed
UASSERT(thread->stop() == true);
// wait() only needs to wait the first time - the other two are no-ops.
UASSERT(thread->wait() == true);
UASSERT(thread->wait() == false);
UASSERT(thread->wait() == false);
// Now that the thread is stopped, we should be able to get the
// return value, and it should be the object itself.
thread_retval = NULL;
UASSERT(thread->getReturnValue(&thread_retval) == true);
UASSERT(thread_retval == thread);
}
delete thread;
}
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: public:
AtomicTestThread(Atomic<u32> &v, Semaphore &trigger) : AtomicTestThread(Atomic<u32> &v, Semaphore &trigger) :
Thread("AtomicTest"), Thread("AtomicTest"),
val(v), val(v),
trigger(trigger) trigger(trigger)
{} {
}
private: private:
void *run() void *run()
{ {
@ -56,6 +152,7 @@ private:
++val; ++val;
return NULL; return NULL;
} }
Atomic<u32> &val; Atomic<u32> &val;
Semaphore &trigger; Semaphore &trigger;
}; };