# HG changeset patch # User rubidium # Date 2008-06-08 15:27:57 # Node ID 31c691398ec3d80f994bc9971ee974b918bb5d59 # Parent 18c70126356dda8d89876bb7aa46e36083152550 (svn r13417) -Fix (r12945, r13413): freeing the ThreadObjects in a manner that hopefully doesn't cause crashes. diff --git a/src/fiber_thread.cpp b/src/fiber_thread.cpp --- a/src/fiber_thread.cpp +++ b/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); } /** diff --git a/src/genworld.cpp b/src/genworld.cpp --- a/src/genworld.cpp +++ b/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 mod /* 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); diff --git a/src/saveload.cpp b/src/saveload.cpp --- a/src/saveload.cpp +++ b/src/saveload.cpp @@ -95,6 +95,7 @@ static void NORETURN SlError(StringID st 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 * 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); diff --git a/src/thread.h b/src/thread.h --- a/src/thread.h +++ b/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; } }; /** diff --git a/src/thread_morphos.cpp b/src/thread_morphos.cpp --- a/src/thread_morphos.cpp +++ b/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() diff --git a/src/thread_none.cpp b/src/thread_none.cpp --- a/src/thread_none.cpp +++ b/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; } diff --git a/src/thread_os2.cpp b/src/thread_os2.cpp --- a/src/thread_os2.cpp +++ b/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; } diff --git a/src/thread_pthread.cpp b/src/thread_pthread.cpp --- a/src/thread_pthread.cpp +++ b/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() diff --git a/src/thread_win32.cpp b/src/thread_win32.cpp --- a/src/thread_win32.cpp +++ b/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()