Fixed Bugzilla bug #205

Removed SDL_KillThread() from the API, as it isn't safe on many platforms.
libsdl-org · Nov 24, 2008 · 3772374 · 3772374
1 parent bde2292
commit 3772374
Show file tree

Hide file tree

Showing 14 changed files with 15 additions and 106 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -54,12 +54,6 @@ SDL_SYS_WaitThread(SDL_Thread * thread)
     return;
 }
 
-void
-SDL_SYS_KillThread(SDL_Thread * thread)
-{
-    return;
-}
-
 #else
 
 #include <signal.h>
@@ -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
@@ -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
@@ -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
@@ -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);
 }