From 37723740459a987dfe9f7f0b64e48d8a302fd57b Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 24 Nov 2008 00:18:42 +0000 Subject: [PATCH] Fixed Bugzilla bug #205 Removed SDL_KillThread() from the API, as it isn't safe on many platforms. --- include/SDL_thread.h | 7 ++++++- src/thread/SDL_systhread.h | 3 --- src/thread/SDL_thread.c | 5 +---- src/thread/beos/SDL_systhread.c | 6 ------ src/thread/dc/SDL_systhread.c | 6 ------ src/thread/generic/SDL_systhread.c | 6 ------ src/thread/irix/SDL_systhread.c | 7 ------- src/thread/os2/SDL_systhread.c | 10 ---------- src/thread/pth/SDL_systhread.c | 7 ------- src/thread/pthread/SDL_systhread.c | 13 ------------- src/thread/riscos/SDL_systhread.c | 16 ---------------- src/thread/win32/SDL_systhread.c | 10 ---------- test/testhread.c | 12 +----------- test/testlock.c | 13 +++++++------ 14 files changed, 15 insertions(+), 106 deletions(-) diff --git a/include/SDL_thread.h b/include/SDL_thread.h index 32b612694..569456f86 100644 --- a/include/SDL_thread.h +++ b/include/SDL_thread.h @@ -122,7 +122,12 @@ extern DECLSPEC Uint32 SDLCALL SDL_GetThreadID(SDL_Thread * thread); */ extern DECLSPEC void SDLCALL SDL_WaitThread(SDL_Thread * thread, int *status); -/* Forcefully kill a thread without worrying about its state */ +/* This function is here for binary compatibility with legacy apps, but + in SDL 1.3 and later, it's a no-op. You cannot forcibly kill a thread + in a safe manner on many platforms. You should instead find a way to + alert your thread that it is time to terminate, and then have it gracefully + exit on its own. Do not ever call this function! + */ extern DECLSPEC void SDLCALL SDL_KillThread(SDL_Thread * thread); diff --git a/src/thread/SDL_systhread.h b/src/thread/SDL_systhread.h index 32c545ceb..c523f869f 100644 --- a/src/thread/SDL_systhread.h +++ b/src/thread/SDL_systhread.h @@ -48,8 +48,5 @@ extern void SDL_SYS_SetupThread(void); */ extern void SDL_SYS_WaitThread(SDL_Thread * thread); -/* This function kills the thread and returns */ -extern void SDL_SYS_KillThread(SDL_Thread * thread); - #endif /* _SDL_systhread_h */ /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/SDL_thread.c b/src/thread/SDL_thread.c index 2c889c429..cc83b9034 100644 --- a/src/thread/SDL_thread.c +++ b/src/thread/SDL_thread.c @@ -306,10 +306,7 @@ SDL_GetThreadID(SDL_Thread * thread) void SDL_KillThread(SDL_Thread * thread) { - if (thread) { - SDL_SYS_KillThread(thread); - SDL_WaitThread(thread, NULL); - } + /* This is a no-op in SDL 1.3 and later. */ } /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/beos/SDL_systhread.c b/src/thread/beos/SDL_systhread.c index f28f6a06e..555df3862 100644 --- a/src/thread/beos/SDL_systhread.c +++ b/src/thread/beos/SDL_systhread.c @@ -98,10 +98,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) wait_for_thread(thread->handle, &the_status); } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - kill_thread(thread->handle); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/dc/SDL_systhread.c b/src/thread/dc/SDL_systhread.c index 49221ffd9..770c44e82 100644 --- a/src/thread/dc/SDL_systhread.c +++ b/src/thread/dc/SDL_systhread.c @@ -58,10 +58,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) thd_wait(thread->handle); } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - thd_destroy(thread->handle); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/generic/SDL_systhread.c b/src/thread/generic/SDL_systhread.c index 3b766911e..1f171b7f0 100644 --- a/src/thread/generic/SDL_systhread.c +++ b/src/thread/generic/SDL_systhread.c @@ -51,10 +51,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) return; } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - return; -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/irix/SDL_systhread.c b/src/thread/irix/SDL_systhread.c index 24ec87965..2db7077a1 100644 --- a/src/thread/irix/SDL_systhread.c +++ b/src/thread/irix/SDL_systhread.c @@ -81,11 +81,4 @@ SDL_WaitThread(SDL_Thread * thread, int *status) } } -/* WARNING: This may not work for systems with 64-bit pid_t */ -void -SDL_KillThread(SDL_Thread * thread) -{ - kill(thread->handle, SIGKILL); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/os2/SDL_systhread.c b/src/thread/os2/SDL_systhread.c index 4dee320e9..24304e0b3 100644 --- a/src/thread/os2/SDL_systhread.c +++ b/src/thread/os2/SDL_systhread.c @@ -102,14 +102,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) DosWaitThread(&tid, DCWW_WAIT); } -/* WARNING: This function is really a last resort. - * Threads should be signaled and then exit by themselves. - * TerminateThread() doesn't perform stack and DLL cleanup. - */ -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - DosKillThread(thread->handle); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/pth/SDL_systhread.c b/src/thread/pth/SDL_systhread.c index df70c9b8a..d4447ddd7 100644 --- a/src/thread/pth/SDL_systhread.c +++ b/src/thread/pth/SDL_systhread.c @@ -101,11 +101,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) pth_join(thread->handle, NULL); } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - pth_cancel(thread->handle); - pth_join(thread->handle, NULL); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/pthread/SDL_systhread.c b/src/thread/pthread/SDL_systhread.c index 660906bb4..14156ece2 100644 --- a/src/thread/pthread/SDL_systhread.c +++ b/src/thread/pthread/SDL_systhread.c @@ -112,17 +112,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) pthread_join(thread->handle, 0); } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ -#ifdef PTHREAD_CANCEL_ASYNCHRONOUS - pthread_cancel(thread->handle); -#else -#ifdef __FREEBSD__ -#warning For some reason, this doesnt actually kill a thread - FreeBSD 3.2 -#endif - pthread_kill(thread->handle, SIGKILL); -#endif -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/riscos/SDL_systhread.c b/src/thread/riscos/SDL_systhread.c index 1b9326e72..69eedbca5 100644 --- a/src/thread/riscos/SDL_systhread.c +++ b/src/thread/riscos/SDL_systhread.c @@ -54,12 +54,6 @@ SDL_SYS_WaitThread(SDL_Thread * thread) return; } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - return; -} - #else #include @@ -143,15 +137,5 @@ SDL_SYS_WaitThread(SDL_Thread * thread) pthread_join(thread->handle, 0); } -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ -#ifdef PTHREAD_CANCEL_ASYNCHRONOUS - pthread_cancel(thread->handle); -#else - pthread_kill(thread->handle, SIGKILL); -#endif -} - #endif /* vi: set ts=4 sw=4 expandtab: */ diff --git a/src/thread/win32/SDL_systhread.c b/src/thread/win32/SDL_systhread.c index 32273c334..8bb26bd5e 100644 --- a/src/thread/win32/SDL_systhread.c +++ b/src/thread/win32/SDL_systhread.c @@ -164,14 +164,4 @@ SDL_SYS_WaitThread(SDL_Thread * thread) CloseHandle(thread->handle); } -/* WARNING: This function is really a last resort. - * Threads should be signaled and then exit by themselves. - * TerminateThread() doesn't perform stack and DLL cleanup. - */ -void -SDL_SYS_KillThread(SDL_Thread * thread) -{ - TerminateThread(thread->handle, FALSE); -} - /* vi: set ts=4 sw=4 expandtab: */ diff --git a/test/testhread.c b/test/testhread.c index c8b9833a4..3ebe0ea58 100644 --- a/test/testhread.c +++ b/test/testhread.c @@ -62,19 +62,9 @@ main(int argc, char *argv[]) alive = 0; SDL_WaitThread(thread, NULL); - alive = 1; - thread = SDL_CreateThread(ThreadFunc, "#2"); - if (thread == NULL) { - fprintf(stderr, "Couldn't create thread: %s\n", SDL_GetError()); - quit(1); - } - SDL_Delay(5 * 1000); - printf("Killing thread #2\n"); - SDL_KillThread(thread); - alive = 1; signal(SIGTERM, killed); - thread = SDL_CreateThread(ThreadFunc, "#3"); + thread = SDL_CreateThread(ThreadFunc, "#2"); if (thread == NULL) { fprintf(stderr, "Couldn't create thread: %s\n", SDL_GetError()); quit(1); diff --git a/test/testlock.c b/test/testlock.c index d31806eba..3a5d19f39 100644 --- a/test/testlock.c +++ b/test/testlock.c @@ -44,8 +44,9 @@ closemutex(int sig) Uint32 id = SDL_ThreadID(); int i; printf("Process %u: Cleaning up...\n", id == mainthread ? 0 : id); + doterminate = 1; for (i = 0; i < 6; ++i) - SDL_KillThread(threads[i]); + SDL_WaitThread(threads[i], NULL); SDL_DestroyMutex(mutex); exit(sig); } @@ -55,7 +56,7 @@ Run(void *data) { if (SDL_ThreadID() == mainthread) signal(SIGTERM, closemutex); - while (1) { + while (!doterminate) { printf("Process %u ready to work\n", SDL_ThreadID()); if (SDL_mutexP(mutex) < 0) { fprintf(stderr, "Couldn't lock mutex: %s", SDL_GetError()); @@ -70,10 +71,10 @@ Run(void *data) } /* If this sleep isn't done, then threads may starve */ SDL_Delay(10); - if (SDL_ThreadID() == mainthread && doterminate) { - printf("Process %u: raising SIGTERM\n", SDL_ThreadID()); - raise(SIGTERM); - } + } + if (SDL_ThreadID() == mainthread && doterminate) { + printf("Process %u: raising SIGTERM\n", SDL_ThreadID()); + raise(SIGTERM); } return (0); }