[SDL] What are threads allowed to do?

Albert Cahalan albert at users.sf.net
Thu Jan 20 16:08:51 PST 2005


Hi Cliff!

> I have an application that uses SDL_SetTimer that dies on
> Hyperthreaded P4s running Windows XP.  Looking at
> SDL_ThreadedTimerCheck, I see this code:

> 			SDL_mutexV(SDL_timer_mutex);
> 			ms = t->cb(t->interval, t->param);
> 			SDL_mutexP(SDL_timer_mutex);

> But without SDL_timer_mutex being held, can't the memory pointed to by
> t be freed?  Shouldn't a copy of *t be made on the stack before the
> lock is released and then that copy's cb, interval and param field be
> used?

Yep, thanks for catching this.

Please try the attached file, to see if it's fixed.

Thanks,
	-Sam Lantinga, Software Engineer, Blizzard Entertainment
-------------- next part --------------
/*
    SDL - Simple DirectMedia Layer
    Copyright (C) 1997, 1998  Sam Lantinga

    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Library General Public
    License as published by the Free Software Foundation; either
    version 2 of the License, or (at your option) any later version.

    This library is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
    Library General Public License for more details.

    You should have received a copy of the GNU Library General Public
    License along with this library; if not, write to the Free
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

    Sam Lantinga
    5635-34 Springhouse Dr.
    Pleasanton, CA 94588 (USA)
    slouken at libsdl.org
*/

#ifdef SAVE_RCSID
static char rcsid =
 "@(#) $Id: SDL_timer.c,v 1.3 2002/03/06 11:23:02 slouken Exp $";
#endif

#include <stdlib.h>
#include <stdio.h>			/* For the definition of NULL */

#include "SDL_error.h"
#include "SDL_timer.h"
#include "SDL_timer_c.h"
#include "SDL_mutex.h"
#include "SDL_systimer.h"

/* #define DEBUG_TIMERS */

int SDL_timer_started = 0;
int SDL_timer_running = 0;

/* Data to handle a single periodic alarm */
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;

struct _SDL_TimerID {
	Uint32 interval;
	SDL_NewTimerCallback cb;
	void *param;
	Uint32 last_alarm;
	struct _SDL_TimerID *next;
};

static SDL_TimerID SDL_timers = NULL;
static Uint32 num_timers = 0;
static SDL_mutex *SDL_timer_mutex;

/* Set whether or not the timer should use a thread.
   This should not be called while the timer subsystem is running.
*/
int SDL_SetTimerThreaded(int value)
{
	int retval;

	if ( SDL_timer_started ) {
		SDL_SetError("Timer already initialized");
		retval = -1;
	} else {
		retval = 0;
		SDL_timer_threaded = value;
	}
	return retval;
}

int SDL_TimerInit(void)
{
	int retval;

	SDL_timer_running = 0;
	SDL_SetTimer(0, NULL);
	retval = 0;
	if ( ! SDL_timer_threaded ) {
		retval = SDL_SYS_TimerInit();
	}
	if ( SDL_timer_threaded ) {
		SDL_timer_mutex = SDL_CreateMutex();
	}
	SDL_timer_started = 1;
	return(retval);
}

void SDL_TimerQuit(void)
{
	SDL_SetTimer(0, NULL);
	if ( SDL_timer_threaded < 2 ) {
		SDL_SYS_TimerQuit();
	}
	if ( SDL_timer_threaded ) {
		SDL_DestroyMutex(SDL_timer_mutex);
	}
	SDL_timer_started = 0;
	SDL_timer_threaded = 0;
}

void SDL_ThreadedTimerCheck(void)
{
	Uint32 now, ms;
	SDL_TimerID t, prev, next;
	int removed;
	SDL_NewTimerCallback callback;
	Uint32 interval;
	void *param;

	now = SDL_GetTicks();

	SDL_mutexP(SDL_timer_mutex);
	for ( prev = NULL, t = SDL_timers; t; t = next ) {
		removed = 0;
		ms = t->interval - SDL_TIMESLICE;
		next = t->next;
		if ( (t->last_alarm < now) && ((now - t->last_alarm) > ms) ) {
			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());
#endif
			callback = t->cb;
			interval = t->interval;
			param = t->param;
			SDL_mutexV(SDL_timer_mutex);
			ms = callback(interval, param);
			SDL_mutexP(SDL_timer_mutex);
			if ( list_changed ) {
				/* Abort, list of timers has been modified */
				break;
			}
			if ( ms != t->interval ) {
				if ( ms ) {
					t->interval = ROUND_RESOLUTION(ms);
				} else { /* Remove the timer from the linked list */
#ifdef DEBUG_TIMERS
					printf("SDL: Removing timer %p\n", t);
#endif
					if ( prev ) {
						prev->next = next;
					} else {
						SDL_timers = next;
					}
					free(t);
					-- num_timers;
					removed = 1;
				}
			}
		}
		/* Don't update prev if the timer has disappeared */
		if ( ! removed ) {
			prev = t;
		}
	}
	SDL_mutexV(SDL_timer_mutex);
}

SDL_TimerID SDL_AddTimer(Uint32 interval, SDL_NewTimerCallback callback, void *param)
{
	SDL_TimerID t;
	if ( ! SDL_timer_mutex ) {
		if ( SDL_timer_started ) {
			SDL_SetError("This platform doesn't support multiple timers");
		} else {
			SDL_SetError("You must call SDL_Init(SDL_INIT_TIMER) first");
		}
		return NULL;
	}
	if ( ! SDL_timer_threaded ) {
		SDL_SetError("Multiple timers require threaded events!");
		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
	SDL_mutexV(SDL_timer_mutex);
	return t;
}

SDL_bool SDL_RemoveTimer(SDL_TimerID id)
{
	SDL_TimerID t, prev = NULL;
	SDL_bool removed;

	removed = SDL_FALSE;
	SDL_mutexP(SDL_timer_mutex);
	/* Look for id in the linked list of timers */
	for (t = SDL_timers; t; prev=t, t = t->next ) {
		if ( t == id ) {
			if(prev) {
				prev->next = t->next;
			} else {
				SDL_timers = t->next;
			}
			free(t);
			-- num_timers;
			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());
#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)
{
	SDL_TimerCallback func = (SDL_TimerCallback) param;
	return (*func)(ms);
}

int SDL_SetTimer(Uint32 ms, SDL_TimerCallback callback)
{
	int retval;

#ifdef DEBUG_TIMERS
	printf("SDL_SetTimer(%d)\n", ms);
#endif
	retval = 0;
	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);
		} else {
			SDL_SYS_StopTimer();
		}
	}
	if ( ms ) {
		if ( SDL_timer_threaded ) {
			retval = (SDL_AddTimer(ms, callback_wrapper,
					       (void *)callback) != NULL);
		} else {
			SDL_timer_running = 1;
			SDL_alarm_interval = ms;
			SDL_alarm_callback = callback;
			retval = SDL_SYS_StartTimer();
		}
	}
	return retval;
}


More information about the SDL mailing list