Fix broken async locking in release build

minetest · Dec 3, 2013 · 5004f31 · 5004f31
1 parent 6cbd1b8
commit 5004f31
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 42 deletions.
diff --git a/src/jthread/jthread.h b/src/jthread/jthread.h
@@ -50,6 +50,13 @@ class JThread
 	bool StopRequested();
 	void *GetReturnValue();
 	bool IsSameThread();
+
+	/*
+	 * Wait for thread to finish
+	 * Note: this does not stop a thread you have to do this on your own
+	 * WARNING: never ever call this on a thread not started or already killed!
+	 */
+	void Wait();
 protected:
 	void ThreadStarted();
 private:
@@ -67,6 +74,8 @@ class JThread
 	static void *TheThread(void *param);
 
 	pthread_t threadid;
+
+	bool started;
 #endif // WIN32
 	void *retval;
 	bool running;

diff --git a/src/jthread/pthread/jevent.cpp b/src/jthread/pthread/jevent.cpp
@@ -27,18 +27,28 @@
 #include <assert.h>
 #include "jthread/jevent.h"
 
+#define UNUSED(expr) do { (void)(expr); } while (0)
+
 Event::Event() {
-	assert(sem_init(&sem, 0, 0) == 0);
+	int sem_init_retval = sem_init(&sem, 0, 0);
+	assert(sem_init_retval == 0);
+	UNUSED(sem_init_retval);
 }
 
 Event::~Event() {
-	assert(sem_destroy(&sem) == 0);
+	int sem_destroy_retval = sem_destroy(&sem);
+	assert(sem_destroy_retval == 0);
+	UNUSED(sem_destroy_retval);
 }
 
 void Event::wait() {
-	assert(sem_wait(&sem) == 0);
+	int sem_wait_retval = sem_wait(&sem);
+	assert(sem_wait_retval == 0);
+	UNUSED(sem_wait_retval);
 }
 
 void Event::signal() {
-	assert(sem_post(&sem) == 0);
+	int sem_post_retval = sem_post(&sem);
+	assert(sem_post_retval == 0);
+	UNUSED(sem_post_retval);
 }
diff --git a/src/jthread/pthread/jmutex.cpp b/src/jthread/pthread/jmutex.cpp
@@ -26,25 +26,33 @@
 */
 #include <assert.h>
 #include "jthread/jmutex.h"
-
+#define UNUSED(expr) do { (void)(expr); } while (0)
 JMutex::JMutex()
 {
-	assert(pthread_mutex_init(&mutex,NULL) == 0);
+	int mutex_init_retval = pthread_mutex_init(&mutex,NULL);
+	assert( mutex_init_retval == 0 );
+	UNUSED(mutex_init_retval);
 }
 
 JMutex::~JMutex()
 {
-	assert(pthread_mutex_destroy(&mutex) == 0);
+	int mutex_dextroy_retval = pthread_mutex_destroy(&mutex);
+	assert( mutex_dextroy_retval == 0 );
+	UNUSED(mutex_dextroy_retval);
 }
 
 int JMutex::Lock()
 {
-	assert(pthread_mutex_lock(&mutex) == 0);
-	return 0;
+	int mutex_lock_retval = pthread_mutex_lock(&mutex);
+	assert( mutex_lock_retval == 0 );
+	return mutex_lock_retval;
+	UNUSED(mutex_lock_retval);
 }
 
 int JMutex::Unlock()
 {
-	assert(pthread_mutex_unlock(&mutex) == 0);
-	return 0;
+	int mutex_unlock_retval = pthread_mutex_unlock(&mutex);
+	assert( mutex_unlock_retval == 0 );
+	return mutex_unlock_retval;
+	UNUSED(mutex_unlock_retval);
 }
diff --git a/src/jthread/pthread/jsemaphore.cpp b/src/jthread/pthread/jsemaphore.cpp
@@ -18,25 +18,35 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 */
 #include <assert.h>
 #include "jthread/jsemaphore.h"
-
+#define UNUSED(expr) do { (void)(expr); } while (0)
 JSemaphore::JSemaphore() {
-	assert(sem_init(&m_semaphore,0,0) == 0);
+	int sem_init_retval = sem_init(&m_semaphore,0,0);
+	assert(sem_init_retval == 0);
+	UNUSED(sem_init_retval);
 }
 
 JSemaphore::~JSemaphore() {
-	assert(sem_destroy(&m_semaphore) == 0);
+	int sem_destroy_retval = sem_destroy(&m_semaphore);
+	assert(sem_destroy_retval == 0);
+	UNUSED(sem_destroy_retval);
 }
 
 JSemaphore::JSemaphore(int initval) {
-	assert(sem_init(&m_semaphore,0,initval) == 0);
+	int sem_init_retval = sem_init(&m_semaphore,0,initval);
+	assert(sem_init_retval == 0);
+	UNUSED(sem_init_retval);
 }
 
 void JSemaphore::Post() {
-	assert(sem_post(&m_semaphore) == 0);
+	int sem_post_retval = sem_post(&m_semaphore);
+	assert(sem_post_retval == 0);
+	UNUSED(sem_post_retval);
 }
 
 void JSemaphore::Wait() {
-	assert(sem_wait(&m_semaphore) == 0);
+	int sem_wait_retval = sem_wait(&m_semaphore);
+	assert(sem_wait_retval == 0);
+	UNUSED(sem_wait_retval);
 }
 
 int JSemaphore::GetValue() {

diff --git a/src/jthread/pthread/jthread.cpp b/src/jthread/pthread/jthread.cpp
@@ -26,15 +26,19 @@
 */
 
 #include "jthread/jthread.h"
+#include <assert.h>
 #include <sys/time.h>
 #include <time.h>
 #include <stdlib.h>
 
+#define UNUSED(expr) do { (void)(expr); } while (0)
+
 JThread::JThread()
 {
 	retval = NULL;
 	requeststop = false;
 	running = false;
+	started = false;
 }
 
 JThread::~JThread()
@@ -48,6 +52,20 @@ void JThread::Stop() {
 	runningmutex.Unlock();
 }
 
+void JThread::Wait() {
+	void* status;
+	runningmutex.Lock();
+	if (started) {
+		runningmutex.Unlock();
+		int pthread_join_retval = pthread_join(threadid,&status);
+		assert(pthread_join_retval == 0);
+		UNUSED(pthread_join_retval);
+		runningmutex.Lock();
+		started = false;
+	}
+	runningmutex.Unlock();
+}
+
 int JThread::Start()
 {
 	int status;
@@ -63,7 +81,7 @@ int JThread::Start()
 
 	pthread_attr_t attr;
 	pthread_attr_init(&attr);
-	pthread_attr_setdetachstate(&attr,PTHREAD_CREATE_DETACHED);
+	//pthread_attr_setdetachstate(&attr,PTHREAD_CREATE_DETACHED);
 
 	continuemutex.Lock();
 	status = pthread_create(&threadid,&attr,TheThread,this);
@@ -89,6 +107,7 @@ int JThread::Start()
 
 		runningmutex.Lock();
 	}
+	started = true;
 	runningmutex.Unlock();
 
 	continuemutex.Unlock();
@@ -100,13 +119,30 @@ int JThread::Start()
 
 int JThread::Kill()
 {
+	void* status;
 	runningmutex.Lock();
 	if (!running)
 	{
+		if (started) {
+			runningmutex.Unlock();
+			int pthread_join_retval = pthread_join(threadid,&status);
+			assert(pthread_join_retval == 0);
+			UNUSED(pthread_join_retval);
+			runningmutex.Lock();
+			started = false;
+		}
 		runningmutex.Unlock();
 		return ERR_JTHREAD_NOTRUNNING;
 	}
 	pthread_cancel(threadid);
+	if (started) {
+		runningmutex.Unlock();
+		int pthread_join_retval = pthread_join(threadid,&status);
+		assert(pthread_join_retval == 0);
+		UNUSED(pthread_join_retval);
+		runningmutex.Lock();
+		started = false;
+	}
 	running = false;
 	runningmutex.Unlock();
 	return 0;

diff --git a/src/jthread/win32/jthread.cpp b/src/jthread/win32/jthread.cpp
@@ -26,7 +26,8 @@
 */
 
 #include "jthread/jthread.h"
-
+#include <assert.h>
+#define UNUSED(expr) do { (void)(expr); } while (0)
 #ifndef _WIN32_WCE
 	#include <process.h>
 #endif // _WIN32_WCE
@@ -49,6 +50,12 @@ void JThread::Stop() {
 	runningmutex.Unlock();
 }
 
+void JThread::Wait() {
+	int WaitForSingleObject_retval = WaitForSingleObject(threadhandle, INFINITE);
+	assert(WaitForSingleObject_retval == 0);
+	UNUSED(WaitForSingleObject_retval);
+}
+
 int JThread::Start()
 {
 	runningmutex.Lock();

diff --git a/src/script/lua_api/l_async_events.h b/src/script/lua_api/l_async_events.h
@@ -17,23 +17,12 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 
-#ifndef C_ASYNC_EVENTS_H_
-#define C_ASYNC_EVENTS_H_
+#ifndef L_ASYNC_EVENTS_H_
+#define L_ASYNC_EVENTS_H_
 
 #include <vector>
 #include <map>
 
-#ifndef _WIN32
-#include <unistd.h>
-#else
-#define _WINSOCKAPI_
-#include <windows.h>
-static unsigned sleep(unsigned seconds) {
-	Sleep(seconds * 1000);
-	return 0;
-}
-#endif
-
 /******************************************************************************/
 /* Includes                                                                   */
 /******************************************************************************/
@@ -93,15 +82,6 @@ class AsyncWorkerThread : public JThread {
 			return worker_thread_wrapper(this);
 	}
 
-	/**
-	 * wait for thread to stop
-	 */
-	void Wait() {
-		while(IsRunning()) {
-			sleep(1);
-		}
-	}
-
 private:
 	/**
 	 * helper function to run a lua script
@@ -237,4 +217,4 @@ class AsyncEngine {
 	JSemaphore m_JobQueueCounter;
 };
 
-#endif /* C_ASYNC_EVENTS_H_ */
+#endif /* L_ASYNC_EVENTS_H_ */