[Scummvm-cvs-logs] CVS: scummvm/common engine.cpp,1.20,1.21 timer.cpp,1.10,1.11 timer.h,1.6,1.7

Max Horn fingolfin at users.sourceforge.net
Thu Jul 3 04:21:30 CEST 2003


Update of /cvsroot/scummvm/scummvm/common
In directory sc8-pr-cvs1:/tmp/cvs-serv18584

Modified Files:
	engine.cpp timer.cpp timer.h 
Log Message:
Timer now uses a mutex, which should make it thread safe (it wasn't before, particuarly bad if timers are implemented via threads), plus this should help in fixing race conditions in classes using class Timer

Index: engine.cpp
===================================================================
RCS file: /cvsroot/scummvm/scummvm/common/engine.cpp,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -d -r1.20 -r1.21
--- engine.cpp	6 Jun 2003 22:51:31 -0000	1.20
+++ engine.cpp	3 Jul 2003 11:18:07 -0000	1.21
@@ -41,7 +41,6 @@
 	g_system = _system;
 	g_mixer = _mixer;
 	_timer = new Timer(this);
-	_timer->init();
 }
 
 Engine::~Engine() {

Index: timer.cpp
===================================================================
RCS file: /cvsroot/scummvm/scummvm/common/timer.cpp,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -d -r1.10 -r1.11
--- timer.cpp	19 Jun 2003 15:14:17 -0000	1.10
+++ timer.cpp	3 Jul 2003 11:18:07 -0000	1.11
@@ -26,17 +26,49 @@
 
 static Timer *g_timer = NULL;
 
-Timer::Timer(Engine * engine) {
-	memset(this,0,sizeof(Timer)); //palmos
-	
-	_initialized = false;
-	_timerRunning = false;
-	_engine = engine;
+Timer::Timer(Engine * engine) :
+	_engine(engine),
+	_mutex(0),
+	_timerHandler(0),
+	_lastTime(0) {
+
+	_mutex = _engine->_system->create_mutex();
+
 	g_timer = this;
+	
+	for (int i = 0; i < MAX_TIMERS; i++) {
+		_timerSlots[i].procedure = NULL;
+		_timerSlots[i].interval = 0;
+		_timerSlots[i].counter = 0;
+	}
+
+	_thisTime = _engine->_system->get_msecs();
+
+	// Set the timer last, after everything has been initialised
+	_engine->_system->set_timer(10, &timer_handler);
+
 }
 
 Timer::~Timer() {
-	release();
+	_engine->_system->set_timer(0, NULL);
+
+	_engine->_system->lock_mutex(_mutex);
+	for (int i = 0; i < MAX_TIMERS; i++) {
+		_timerSlots[i].procedure = NULL;
+		_timerSlots[i].interval = 0;
+		_timerSlots[i].counter = 0;
+	}
+	_engine->_system->unlock_mutex(_mutex);
+
+	// FIXME: There is still a potential race condition here, depending on how
+	// the system backend implements set_timer: If timers are done using
+	// threads, and if set_timer does *not* gurantee that after it terminates
+	// that timer thread is not run anymore, we are fine. However, if the timer
+	// is still running in parallel to this destructor, then it might be that
+	// it is still waiting for the _mutex. So, again depending on the backend,
+	// we might end up unlocking the mutex then immediately deleting it, while
+	// the timer thread is about to lock it.
+	_engine->_system->delete_mutex(_mutex);
 }
 
 int Timer::timer_handler(int t) {
@@ -48,77 +80,32 @@
 int Timer::handler(int t) {
 	uint32 interval, l;
 
-	if (_timerRunning) {
-		_lastTime = _thisTime;
-		_thisTime = _engine->_system->get_msecs();
-		interval = 1000 * (_thisTime - _lastTime);
-
-		for (l = 0; l < MAX_TIMERS; l++) {
-			if ((_timerSlots[l].procedure) && (_timerSlots[l].interval > 0)) {
-				_timerSlots[l].counter -= interval;
-				if (_timerSlots[l].counter <= 0) {
-					_timerSlots[l].counter += _timerSlots[l].interval;
-					_timerSlots[l].procedure (_engine);
-				}
-			}
-		}
-	}
-
-	return t;
-}
-
-bool Timer::init() {
-	int32 l;
-
-	if (_engine->_system == NULL) {
-		warning("Timer: OSystem not initialized!");
-		return false;
-	}
+	_engine->_system->lock_mutex(_mutex);
 
-	if (_initialized) 
-		return true;
+	_lastTime = _thisTime;
+	_thisTime = _engine->_system->get_msecs();
+	interval = 1000 * (_thisTime - _lastTime);
 
 	for (l = 0; l < MAX_TIMERS; l++) {
-		_timerSlots[l].procedure = NULL;
-		_timerSlots[l].interval = 0;
-		_timerSlots[l].counter = 0;
+		if ((_timerSlots[l].procedure) && (_timerSlots[l].interval > 0)) {
+			_timerSlots[l].counter -= interval;
+			if (_timerSlots[l].counter <= 0) {
+				_timerSlots[l].counter += _timerSlots[l].interval;
+				_timerSlots[l].procedure (_engine);
+			}
+		}
 	}
 
-	_thisTime = _engine->_system->get_msecs();
-	_engine->_system->set_timer(10, &timer_handler);
-
-	_timerRunning = true;
-	_initialized = true;
-	return true;
-}
-
-void Timer::release() {
-	int32 l;
-
-	if (_initialized == false) 
-		return;
-
-	_timerRunning = false;
-	_engine->_system->set_timer(0, NULL);
-	_initialized = false;
+	_engine->_system->unlock_mutex(_mutex);
 
-	for (l = 0; l < MAX_TIMERS; l++) {
-		_timerSlots[l].procedure = NULL;
-		_timerSlots[l].interval = 0;
-		_timerSlots[l].counter = 0;
-	}
+	return t;
 }
 
 bool Timer::installProcedure (TimerProc procedure, int32 interval) {
 	int32 l;
 	bool found = false;
 
-	if (_initialized == false) {
-		warning("Timer: is not initialized!");
-		return false;
-	}
-
-	_timerRunning = false;
+	_engine->_system->lock_mutex(_mutex);
 	for (l = 0; l < MAX_TIMERS; l++) {
 		if (!_timerSlots[l].procedure) {
 			_timerSlots[l].procedure = procedure;
@@ -128,25 +115,18 @@
 			break;
 		}
 	}
+	_engine->_system->unlock_mutex(_mutex);
 
-	_timerRunning = true;
-	if (!found)	{
-		warning("Can't find free slot!");
-		return false;
-	}
+	if (!found)
+		warning("Couldn't find free timer slot!");
 
-	return true;
+	return found;
 }
 
 void Timer::releaseProcedure (TimerProc procedure) {
 	int32 l;
 
-	if (_initialized == false) {
-		warning("Timer: is not initialized!");
-		return;
-	}
-
-	_timerRunning = false;
+	_engine->_system->lock_mutex(_mutex);
 	for (l = 0; l < MAX_TIMERS; l++) {
 		if (_timerSlots[l].procedure == procedure) {
 			_timerSlots[l].procedure = 0;
@@ -154,7 +134,7 @@
 			_timerSlots[l].counter = 0;
 		}
 	}
-	_timerRunning = true;
+	_engine->_system->unlock_mutex(_mutex);
 }
 
 #endif

Index: timer.h
===================================================================
RCS file: /cvsroot/scummvm/scummvm/common/timer.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- timer.h	29 May 2003 22:34:35 -0000	1.6
+++ timer.h	3 Jul 2003 11:18:07 -0000	1.7
@@ -36,8 +36,7 @@
 
 private:
 	Engine *_engine;
-	bool _initialized;
-	bool _timerRunning;
+	void *_mutex;
 	void *_timerHandler;
 	int32 _thisTime;
 	int32 _lastTime;
@@ -49,11 +48,9 @@
 	} _timerSlots[MAX_TIMERS];
 
 public:
-	  Timer(Engine *engine);
-	 ~Timer();
+	Timer(Engine *engine);
+	~Timer();
 
-	bool init();
-	void release();
 	bool installProcedure(TimerProc procedure, int32 interval);
 	void releaseProcedure(TimerProc procedure);
 





More information about the Scummvm-git-logs mailing list