(svn r13417) -Fix (r12945, r13413): freeing the ThreadObjects in a manner that hopefully doesn't cause crashes.

src/fiber_thread.cpp

@@ -32,7 +32,7 @@ public:
 	{
 		this->m_sem = ThreadSemaphore::New();
 		/* Create a thread and start stFiberProc */
-		this->m_thread = ThreadObject::New(&stFiberProc, this, NULL);
+		this->m_thread = ThreadObject::New(&stFiberProc, this);
 	}
 
 	/**

src/genworld.cpp

@@ -156,7 +156,6 @@ static void _GenerateWorld(void *arg)
 		/* Show all vital windows again, because we have hidden them */
 		if (_gw.threaded && _game_mode != GM_MENU) ShowVitalWindows();
 		_gw.active   = false;
-		_gw.thread   = NULL;
 		_gw.proc     = NULL;
 		_gw.threaded = false;
 
@@ -199,6 +198,7 @@ void WaitTillGeneratedWorld()
 	if (_gw.thread == NULL) return;
 	_gw.quit_thread = true;
 	_gw.thread->Join();
+	delete _gw.thread;
 	_gw.thread   = NULL;
 	_gw.threaded = false;
 }
@@ -233,9 +233,7 @@ void HandleGeneratingWorldAbortion()
 	/* Show all vital windows again, because we have hidden them */
 	if (_gw.threaded && _game_mode != GM_MENU) ShowVitalWindows();
 
-	ThreadObject *thread = _gw.thread;
 	_gw.active   = false;
-	_gw.thread   = NULL;
 	_gw.proc     = NULL;
 	_gw.abortp   = NULL;
 	_gw.threaded = false;
@@ -243,7 +241,7 @@ void HandleGeneratingWorldAbortion()
 	DeleteWindowById(WC_GENERATE_PROGRESS_WINDOW, 0);
 	MarkWholeScreenDirty();
 
-	thread->Exit();
+	_gw.thread->Exit();
 }
 
 /**
@@ -287,8 +285,14 @@ void GenerateWorld(GenerateWorldMode mode, uint size_x, uint size_y)
 	/* Create toolbars */
 	SetupColorsAndInitialWindow();
 
+	if (_gw.thread != NULL) {
+		_gw.thread->Join();
+		delete _gw.thread;
+		_gw.thread = NULL;
+	}
+
 	if (_network_dedicated ||
-	    (_gw.thread = ThreadObject::New(&_GenerateWorld, NULL, &ThreadObject::TerminateCleanup)) == NULL) {
+	    (_gw.thread = ThreadObject::New(&_GenerateWorld, NULL)) == NULL) {
 		DEBUG(misc, 1, "Cannot create genworld thread, reverting to single-threaded mode");
 		_gw.threaded = false;
 		_GenerateWorld(NULL);

src/saveload.cpp

@@ -95,6 +95,7 @@ static void NORETURN SlError(StringID string, const char *extra_msg = NULL)
 
 typedef void (*AsyncSaveFinishProc)();
 static AsyncSaveFinishProc _async_save_finish = NULL;
+static ThreadObject *_save_thread;
 
 /**
  * Called by save thread to tell we finished saving.
@@ -117,6 +118,12 @@ void ProcessAsyncSaveFinish()
 	_async_save_finish();
 
 	_async_save_finish = NULL;
+
+	if (_save_thread != NULL) {
+		_save_thread->Join();
+		delete _save_thread;
+		_save_thread = NULL;
+	}
 }
 
 /**
@@ -1545,8 +1552,6 @@ static void SaveFileError()
 	SaveFileDone();
 }
 
-static ThreadObject *_save_thread;
-
 /** We have written the whole game into memory, _Savegame_pool, now find
  * and appropiate compressor and start writing to file.
  */
@@ -1617,6 +1622,7 @@ void WaitTillSaved()
 	if (_save_thread == NULL) return;
 
 	_save_thread->Join();
+	delete _save_thread;
 	_save_thread = NULL;
 }
 
@@ -1695,7 +1701,7 @@ SaveOrLoadResult SaveOrLoad(const char *filename, int mode, Subdirectory sb)
 
 			SaveFileStart();
 			if (_network_server ||
-						(_save_thread = ThreadObject::New(&SaveFileToDiskThread, NULL, &ThreadObject::TerminateCleanup)) == NULL) {
+						(_save_thread = ThreadObject::New(&SaveFileToDiskThread, NULL)) == NULL) {
 				if (!_network_server) DEBUG(sl, 1, "Cannot create savegame thread, reverting to single-threaded mode...");
 
 				SaveOrLoadResult result = SaveFileToDisk(false);

src/thread.h

@@ -6,7 +6,6 @@
 #define THREAD_H
 
 typedef void (*OTTDThreadFunc)(void *);
-typedef void (*OTTDThreadTerminateFunc)(class ThreadObject *self);
 
 /**
  * A Thread Object which works on all our supported OSes.
@@ -57,10 +56,9 @@ public:
 	 *  with optinal params.
 	 * @param proc The procedure to call inside the thread.
 	 * @param param The params to give with 'proc'.
-	 * @param terminate_func The function (or NULL) to call when the thread terminates.
 	 * @return True if the thread was started correctly.
 	 */
-	static ThreadObject *New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func);
+	static ThreadObject *New(OTTDThreadFunc proc, void *param);
 
 	/**
 	 * Convert the current thread to a new ThreadObject.
@@ -73,12 +71,6 @@ public:
 	 * @return The thread ID of the current active thread.
 	 */
 	static uint CurrentId();
-
-	/**
-	 * A OTTDThreadTerminateFunc, which cleans up the thread itself
-	 *  at termination of the thread (so it becomes self-managed).
-	 */
-	static void TerminateCleanup(ThreadObject *self) { delete self; }
 };
 
 /**

src/thread_morphos.cpp

@@ -57,7 +57,6 @@ void KPutStr(CONST_STRPTR format)
 class ThreadObject_MorphOS : public ThreadObject {
 private:
 	APTR m_thr;                  ///< System thread identifier.
-	OTTDThreadTerminateFunc m_terminate_func; ///< Function to call on thread termination.
 	struct MsgPort *m_replyport;
 	struct OTTDThreadStartupMessage m_msg;
 
@@ -65,9 +64,7 @@ public:
 	/**
 	 * Create a sub process and start it, calling proc(param).
 	 */
-	ThreadObject_MorphOS(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func) :
-		m_thr(0),
-		m_terminate_func(terminate_func)
+	ThreadObject_MorphOS(OTTDThreadFunc proc, void *param) : m_thr(0)
 	{
 		struct Task *parent;
 
@@ -114,9 +111,7 @@ public:
 	/**
 	 * Create a thread and attach current thread to it.
 	 */
-	ThreadObject_MorphOS() :
-		m_thr(0),
-		m_terminate_func(NULL)
+	ThreadObject_MorphOS() : m_thr(0)
 	{
 		m_thr = FindTask(NULL);
 	}
@@ -215,14 +210,12 @@ private:
 
 		/*  Quit the child, exec.library will reply the startup msg internally. */
 		KPutStr("[Child] Done.\n");
-
-		if (this->terminate_func != NULL) this->terminate_func(this);
 	}
 };
 
-/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func)
+/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param)
 {
-	return new ThreadObject_MorphOS(proc, param, terminate_func);
+	return new ThreadObject_MorphOS(proc, param);
 }
 
 /* static */ ThreadObject *ThreadObject::AttachCurrent()

src/thread_none.cpp

@@ -6,7 +6,7 @@
 #include "thread.h"
 #include "fiber.hpp"
 
-/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func)
+/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param)
 {
 	return NULL;
 }

src/thread_os2.cpp

@@ -59,7 +59,7 @@ void OTTDExitThread()
 
 #endif
 
-/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func)
+/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param)
 {
 	return NULL;
 }

src/thread_pthread.cpp

@@ -22,18 +22,16 @@ private:
 	bool      m_attached;        ///< True if the ThreadObject was attached to an existing thread.
 	sem_t     m_sem_start;       ///< Here the new thread waits before it starts.
 	sem_t     m_sem_stop;        ///< Here the other thread can wait for this thread to end.
-	OTTDThreadTerminateFunc m_terminate_func; ///< Function to call on thread termination.
 
 public:
 	/**
 	 * Create a pthread and start it, calling proc(param).
 	 */
-	ThreadObject_pthread(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func) :
+	ThreadObject_pthread(OTTDThreadFunc proc, void *param) :
 		m_thr(0),
 		m_proc(proc),
 		m_param(param),
-		m_attached(false),
-		m_terminate_func(terminate_func)
+		m_attached(false)
 	{
 		sem_init(&m_sem_start, 0, 0);
 		sem_init(&m_sem_stop, 0, 0);
@@ -49,8 +47,7 @@ public:
 		m_thr(0),
 		m_proc(NULL),
 		m_param(0),
-		m_attached(true),
-		m_terminate_func(NULL)
+		m_attached(true)
 	{
 		sem_init(&m_sem_start, 0, 0);
 		sem_init(&m_sem_stop, 0, 0);
@@ -145,14 +142,12 @@ private:
 
 		/* Notify threads waiting for our completion */
 		sem_post(&m_sem_stop);
-
-		if (this->m_terminate_func != NULL) this->m_terminate_func(this);
 	}
 };
 
-/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func)
+/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param)
 {
-	return new ThreadObject_pthread(proc, param, terminate_func);
+	return new ThreadObject_pthread(proc, param);
 }
 
 /* static */ ThreadObject *ThreadObject::AttachCurrent()

src/thread_win32.cpp

@@ -20,19 +20,17 @@ private:
 	OTTDThreadFunc m_proc;
 	void     *m_param;
 	bool     m_attached;
-	OTTDThreadTerminateFunc m_terminate_func;
 
 public:
 	/**
 	 * Create a win32 thread and start it, calling proc(param).
 	 */
-	ThreadObject_Win32(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func) :
+	ThreadObject_Win32(OTTDThreadFunc proc, void *param) :
 		m_id_thr(0),
 		m_h_thr(NULL),
 		m_proc(proc),
 		m_param(param),
-		m_attached(false),
-		m_terminate_func(terminate_func)
+		m_attached(false)
 	{
 		m_h_thr = (HANDLE)_beginthreadex(NULL, 0, &stThreadProc, this, CREATE_SUSPENDED, &m_id_thr);
 		if (m_h_thr == NULL) return;
@@ -47,8 +45,7 @@ public:
 		m_h_thr(NULL),
 		m_proc(NULL),
 		m_param(NULL),
-		m_attached(false),
-		m_terminate_func(NULL)
+		m_attached(false)
 	{
 		BOOL ret = DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &m_h_thr, 0, FALSE, DUPLICATE_SAME_ACCESS);
 		if (!ret) return;
@@ -133,14 +130,12 @@ private:
 			m_proc(m_param);
 		} catch (...) {
 		}
-
-		if (this->m_terminate_func != NULL) this->m_terminate_func(this);
 	}
 };
 
-/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param, OTTDThreadTerminateFunc terminate_func)
+/* static */ ThreadObject *ThreadObject::New(OTTDThreadFunc proc, void *param)
 {
-	return new ThreadObject_Win32(proc, param, terminate_func);
+	return new ThreadObject_Win32(proc, param);
 }
 
 /* static */ ThreadObject* ThreadObject::AttachCurrent()