Fix various problems with the timer code.
* SDL_timer_running wasn't always updated correctly.
* Fixed occasional crash in SDL_SetTimer() when clearing threaded timers
* It was possible to get both the timer thread and event thread running
* Other misc. cleanup
--- a/include/SDL_timer.h Wed Jan 12 19:38:24 2005 +0000
+++ b/include/SDL_timer.h Thu Jan 13 23:24:56 2005 +0000
@@ -81,6 +81,8 @@
* in the same program, as it is implemented using setitimer(). You also
* should not use this function in multi-threaded applications as signals
* to multi-threaded apps have undefined behavior in some implementations.
+ *
+ * This function returns 0 if successful, or -1 if there was an error.
*/
extern DECLSPEC int SDLCALL SDL_SetTimer(Uint32 interval, SDL_TimerCallback callback);
--- a/src/events/SDL_events.c Wed Jan 12 19:38:24 2005 +0000
+++ b/src/events/SDL_events.c Thu Jan 13 23:24:56 2005 +0000
@@ -91,7 +91,6 @@
static int SDL_GobbleEvents(void *unused)
{
- SDL_SetTimerThreaded(2);
event_thread = SDL_ThreadID();
while ( SDL_EventQ.active ) {
SDL_VideoDevice *video = current_video;
@@ -114,7 +113,7 @@
/* Give up the CPU for the rest of our timeslice */
SDL_EventLock.safe = 1;
- if( SDL_timer_running ) {
+ if ( SDL_timer_running ) {
SDL_ThreadedTimerCheck();
}
SDL_Delay(1);
@@ -162,6 +161,8 @@
}
SDL_EventLock.safe = 0;
+ /* The event thread will handle timers too */
+ SDL_SetTimerThreaded(2);
SDL_EventThread = SDL_CreateThread(SDL_GobbleEvents, NULL);
if ( SDL_EventThread == NULL ) {
return(-1);
--- a/src/timer/SDL_timer.c Wed Jan 12 19:38:24 2005 +0000
+++ b/src/timer/SDL_timer.c Thu Jan 13 23:24:56 2005 +0000
@@ -45,8 +45,6 @@
Uint32 SDL_alarm_interval = 0;
SDL_TimerCallback SDL_alarm_callback;
-static volatile SDL_bool list_changed = SDL_FALSE;
-
/* Data used for a thread-based timer */
static int SDL_timer_threaded = 0;
@@ -59,8 +57,8 @@
};
static SDL_TimerID SDL_timers = NULL;
-static Uint32 num_timers = 0;
static SDL_mutex *SDL_timer_mutex;
+static volatile SDL_bool list_changed = SDL_FALSE;
/* Set whether or not the timer should use a thread.
This should not be called while the timer subsystem is running.
@@ -83,16 +81,19 @@
{
int retval;
- SDL_timer_running = 0;
- SDL_SetTimer(0, NULL);
retval = 0;
+ if ( SDL_timer_started ) {
+ SDL_TimerQuit();
+ }
if ( ! SDL_timer_threaded ) {
retval = SDL_SYS_TimerInit();
}
if ( SDL_timer_threaded ) {
SDL_timer_mutex = SDL_CreateMutex();
}
- SDL_timer_started = 1;
+ if ( retval == 0 ) {
+ SDL_timer_started = 1;
+ }
return(retval);
}
@@ -113,43 +114,41 @@
{
Uint32 now, ms;
SDL_TimerID t, prev, next;
- int removed;
- SDL_NewTimerCallback callback;
- Uint32 interval;
- void *param;
-
- now = SDL_GetTicks();
+ SDL_bool removed;
SDL_mutexP(SDL_timer_mutex);
+ list_changed = SDL_FALSE;
+ now = SDL_GetTicks();
for ( prev = NULL, t = SDL_timers; t; t = next ) {
- removed = 0;
+ removed = SDL_FALSE;
ms = t->interval - SDL_TIMESLICE;
next = t->next;
- if ( (t->last_alarm < now) && ((now - t->last_alarm) > ms) ) {
+ if ( (int)(now - t->last_alarm) > (int)ms ) {
+ struct _SDL_TimerID timer;
+
if ( (now - t->last_alarm) < t->interval ) {
t->last_alarm += t->interval;
} else {
t->last_alarm = now;
}
- list_changed = SDL_FALSE;
#ifdef DEBUG_TIMERS
printf("Executing timer %p (thread = %d)\n",
- t, SDL_ThreadID());
+ t, SDL_ThreadID());
#endif
- callback = t->cb;
- interval = t->interval;
- param = t->param;
+ timer = *t;
SDL_mutexV(SDL_timer_mutex);
- ms = callback(interval, param);
+ ms = timer.cb(timer.interval, timer.param);
SDL_mutexP(SDL_timer_mutex);
if ( list_changed ) {
- /* Abort, list of timers has been modified */
+ /* Abort, list of timers modified */
+ /* FIXME: what if ms was changed? */
break;
}
if ( ms != t->interval ) {
if ( ms ) {
t->interval = ROUND_RESOLUTION(ms);
- } else { /* Remove the timer from the linked list */
+ } else {
+ /* Remove timer from the list */
#ifdef DEBUG_TIMERS
printf("SDL: Removing timer %p\n", t);
#endif
@@ -159,8 +158,8 @@
SDL_timers = next;
}
free(t);
- -- num_timers;
- removed = 1;
+ --SDL_timer_running;
+ removed = SDL_TRUE;
}
}
}
@@ -172,6 +171,26 @@
SDL_mutexV(SDL_timer_mutex);
}
+static SDL_TimerID SDL_AddTimerInternal(Uint32 interval, SDL_NewTimerCallback callback, void *param)
+{
+ SDL_TimerID t;
+ t = (SDL_TimerID) malloc(sizeof(struct _SDL_TimerID));
+ if ( t ) {
+ t->interval = ROUND_RESOLUTION(interval);
+ t->cb = callback;
+ t->param = param;
+ t->last_alarm = SDL_GetTicks();
+ t->next = SDL_timers;
+ SDL_timers = t;
+ ++SDL_timer_running;
+ list_changed = SDL_TRUE;
+ }
+#ifdef DEBUG_TIMERS
+ printf("SDL_AddTimer(%d) = %08x num_timers = %d\n", interval, (Uint32)t, SDL_timer_running);
+#endif
+ return t;
+}
+
SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_NewTimerCallback callback, void *param)
{
SDL_TimerID t;
@@ -188,21 +207,7 @@
return NULL;
}
SDL_mutexP(SDL_timer_mutex);
- t = (SDL_TimerID) malloc(sizeof(struct _SDL_TimerID));
- if ( t ) {
- t->interval = ROUND_RESOLUTION(interval);
- t->cb = callback;
- t->param = param;
- t->last_alarm = SDL_GetTicks();
- t->next = SDL_timers;
- SDL_timers = t;
- ++ num_timers;
- list_changed = SDL_TRUE;
- SDL_timer_running = 1;
- }
-#ifdef DEBUG_TIMERS
- printf("SDL_AddTimer(%d) = %08x num_timers = %d\n", interval, (Uint32)t, num_timers);
-#endif
+ t = SDL_AddTimerInternal(interval, callback, param);
SDL_mutexV(SDL_timer_mutex);
return t;
}
@@ -223,34 +228,19 @@
SDL_timers = t->next;
}
free(t);
- -- num_timers;
+ --SDL_timer_running;
removed = SDL_TRUE;
list_changed = SDL_TRUE;
break;
}
}
#ifdef DEBUG_TIMERS
- printf("SDL_RemoveTimer(%08x) = %d num_timers = %d thread = %d\n", (Uint32)id, removed, num_timers, SDL_ThreadID());
+ printf("SDL_RemoveTimer(%08x) = %d num_timers = %d thread = %d\n", (Uint32)id, removed, SDL_timer_running, SDL_ThreadID());
#endif
SDL_mutexV(SDL_timer_mutex);
return removed;
}
-static void SDL_RemoveAllTimers(SDL_TimerID t)
-{
- SDL_TimerID freeme;
-
- /* Changed to non-recursive implementation.
- The recursive implementation is elegant, but subject to
- stack overflow if there are lots and lots of timers.
- */
- while ( t ) {
- freeme = t;
- t = t->next;
- free(freeme);
- }
-}
-
/* Old style callback functions are wrapped through this */
static Uint32 callback_wrapper(Uint32 ms, void *param)
{
@@ -266,21 +256,29 @@
printf("SDL_SetTimer(%d)\n", ms);
#endif
retval = 0;
+
+ if ( SDL_timer_threaded ) {
+ SDL_mutexP(SDL_timer_mutex);
+ }
if ( SDL_timer_running ) { /* Stop any currently running timer */
- SDL_timer_running = 0;
if ( SDL_timer_threaded ) {
- SDL_mutexP(SDL_timer_mutex);
- SDL_RemoveAllTimers(SDL_timers);
- SDL_timers = NULL;
- SDL_mutexV(SDL_timer_mutex);
+ while ( SDL_timers ) {
+ SDL_TimerID freeme = SDL_timers;
+ SDL_timers = SDL_timers->next;
+ free(freeme);
+ }
+ SDL_timer_running = 0;
+ list_changed = SDL_TRUE;
} else {
SDL_SYS_StopTimer();
+ SDL_timer_running = 0;
}
}
if ( ms ) {
if ( SDL_timer_threaded ) {
- retval = (SDL_AddTimer(ms, callback_wrapper,
- (void *)callback) != NULL);
+ if ( SDL_AddTimerInternal(ms, callback_wrapper, (void *)callback) == NULL ) {
+ retval = -1;
+ }
} else {
SDL_timer_running = 1;
SDL_alarm_interval = ms;
@@ -288,5 +286,9 @@
retval = SDL_SYS_StartTimer();
}
}
+ if ( SDL_timer_threaded ) {
+ SDL_mutexP(SDL_timer_mutex);
+ }
+
return retval;
}