Fixed race condition in MPU401 shutdown

svn-id: r8432

8 files changed, 43 insertions, 21 deletions

diff --git a/backends/midi/coreaudio.cpp b/backends/midi/coreaudio.cpp
index 47561f4f96..65f0e0274f 100644
--- a/backends/midi/coreaudio.cpp
+++ b/backends/midi/coreaudio.cpp
@@ -90,6 +90,8 @@ int MidiDriver_CORE::open() {
 }
 
 void MidiDriver_CORE::close() {
+	MidiDriver_MPU401::close();
+
 	// Stop the output
 	AudioOutputUnitStop(au_output);
 
diff --git a/backends/midi/morphos.cpp b/backends/midi/morphos.cpp
index b4f5fd4eff..20dd409fca 100644
--- a/backends/midi/morphos.cpp
+++ b/backends/midi/morphos.cpp
@@ -68,6 +68,7 @@ int MidiDriver_ETUDE::open()
 
 void MidiDriver_ETUDE::close()
 {
+	MidiDriver_MPU401::close();
 	exit_morphos_music();
 	_isOpen = false;
 }
diff --git a/backends/midi/null.cpp b/backends/midi/null.cpp
index 062bcea31c..9366bc7f1c 100644
--- a/backends/midi/null.cpp
+++ b/backends/midi/null.cpp
@@ -26,7 +26,6 @@
 class MidiDriver_NULL : public MidiDriver_MPU401 {
 public:
 	int open();
-	void close() { }
 	void send(uint32 b) { }
 };
 
diff --git a/backends/midi/quicktime.cpp b/backends/midi/quicktime.cpp
index e61e6409f2..928705ba71 100644
--- a/backends/midi/quicktime.cpp
+++ b/backends/midi/quicktime.cpp
@@ -115,6 +115,8 @@ bail:
 
 void MidiDriver_QT::close()
 {
+	MidiDriver_MPU401::close();
+
 	for (int i = 0; i < 15; i++) {
 		if (qtNoteChannel[i] != NULL)
 			NADisposeNoteChannel(qtNoteAllocator, qtNoteChannel[i]);
diff --git a/backends/midi/seq.cpp b/backends/midi/seq.cpp
index fc7777ece2..2fab845887 100644
--- a/backends/midi/seq.cpp
+++ b/backends/midi/seq.cpp
@@ -87,6 +87,7 @@ int MidiDriver_SEQ::open() {
 }
 
 void MidiDriver_SEQ::close() {
+	MidiDriver_MPU401::close();
 	::close(device);
 	_isOpen = false;
 }
diff --git a/backends/midi/windows.cpp b/backends/midi/windows.cpp
index c4ebcfd9ce..319997d6c5 100644
--- a/backends/midi/windows.cpp
+++ b/backends/midi/windows.cpp
@@ -68,6 +68,7 @@ void MidiDriver_WIN::close() {
 	if (!_isOpen)
 		return;
 	_isOpen = false;
+	MidiDriver_MPU401::close();
 	midiOutUnprepareHeader (_mo, &_streamHeader, sizeof (_streamHeader));
 	check_error(midiOutClose(_mo));
 	CloseHandle (_streamEvent);
diff --git a/sound/mpu401.cpp b/sound/mpu401.cpp
index cd435d5d95..39bfdef482 100644
--- a/sound/mpu401.cpp
+++ b/sound/mpu401.cpp
@@ -76,18 +76,29 @@ const char *MidiDriver::getErrorName(int error_code) {
 	return midi_errors[error_code];
 }
 
-MidiDriver_MPU401::MidiDriver_MPU401() : MidiDriver() {
-	uint i;
-	
-	_started_thread = false;	// palmos
-	_timer_proc = NULL;			// palmos
-	_timer_param = NULL;		// palmos
+MidiDriver_MPU401::MidiDriver_MPU401() :
+	MidiDriver(),
+	_started_thread (false),
+	_timer_proc (0),
+	_timer_param (0),
+	_mutex (0)
+{
 	
+	uint i;
 	for (i = 0; i < ARRAYSIZE(_midi_channels); ++i) {
 		_midi_channels [i].init (this, i);
 	}
 }
 
+void MidiDriver_MPU401::close() {
+	if (_mutex) {
+		_started_thread = false;
+		g_system->lock_mutex (_mutex); // Wait for the timer thread to shutdown.
+		g_system->unlock_mutex (_mutex);
+		g_system->delete_mutex (_mutex);
+	}
+}
+
 MidiChannel *MidiDriver_MPU401::allocateChannel() {
 	MidiChannel_MPU401 *chan;
 	uint i;
@@ -106,40 +117,45 @@ MidiChannel *MidiDriver_MPU401::allocateChannel() {
 
 void MidiDriver_MPU401::setTimerCallback (void *timer_param, void (*timer_proc) (void *)) {
 	if (!_timer_proc || !timer_proc) {
-		_timer_proc = (TimerCallback *) timer_proc;
+		_timer_proc = timer_proc;
 		_timer_param = timer_param;
 		if (!_started_thread && timer_proc) {
 			// TODO: This is the only place in ScummVM where create_thread is
 			// being used. And it's used for a timer like thread. So if we
 			// could convert it to use the timer API instead, we could get
 			// rid of create_thread completely.
+			_mutex = g_system->create_mutex();
+			_started_thread = true;
 			g_system->create_thread(midi_driver_thread, this);
 		}
-		_started_thread = true;
 	}
 }
 
 #if !defined(__MORPHOS__) && !defined(__PALM_OS__)
+typedef void (*TimerCallback) (void*);
+
 int MidiDriver_MPU401::midi_driver_thread(void *param) {
-	volatile MidiDriver_MPU401 *mid = (MidiDriver_MPU401 *)param;
+	MidiDriver_MPU401 *mid = (MidiDriver_MPU401 *)param;
 	int old_time, cur_time;
 
-	old_time = g_system->get_msecs();
+	// Grab the MidiDriver's mutex. When the MidiDriver
+	// shuts down, it will wait on that mutex until we've
+	// detected the shutdown and quit looping.
+	g_system->lock_mutex (mid->_mutex);
 
-	for (;;) {
+	old_time = g_system->get_msecs();
+	while (mid->_started_thread) {
 		g_system->delay_msecs(10);
 
 		cur_time = g_system->get_msecs();
 		while (old_time < cur_time) {
 			old_time += 10;
-			// Don't use mid->_se_on_timer()
-			// We must come in through IMuseMonitor to protect
-			// against conflicts with script access to IMuse.
 			if (mid->_timer_proc)
-				(*(mid->_timer_proc)) (mid->_timer_param);
+				(*(TimerCallback)(mid->_timer_proc)) (mid->_timer_param);
 		}
 	}
 
+	g_system->unlock_mutex (mid->_mutex);
 	return 0;
 }
 #endif
diff --git a/sound/mpu401.h b/sound/mpu401.h
index ecf8cd0919..a85bb6af0c 100644
--- a/sound/mpu401.h
+++ b/sound/mpu401.h
@@ -30,8 +30,6 @@
 //
 ////////////////////////////////////////
 
-typedef void TimerCallback (void *);
-
 class MidiDriver_MPU401;
 
 class MidiChannel_MPU401 : public MidiChannel {
@@ -78,8 +76,9 @@ public:
 class MidiDriver_MPU401 : public MidiDriver {
 private:
 	MidiChannel_MPU401 _midi_channels [16];
-	bool _started_thread;
-	TimerCallback *_timer_proc;
+	volatile bool _started_thread;
+	void *_mutex; // Concurrent shutdown barrier
+	volatile void *_timer_proc;
 	void *_timer_param;
 
 	static int midi_driver_thread (void *param);
@@ -87,8 +86,9 @@ private:
 public:
 	MidiDriver_MPU401();
 
+	virtual void close();
 	void setTimerCallback(void *timer_param, void (*timer_proc) (void *));
-	uint32 getBaseTempo(void) { return 10000; } // 0x4A0000; } // Now referenced in microseconds between callbacks
+	uint32 getBaseTempo(void) { return 10000; }
 
 	MidiChannel *allocateChannel();
 	MidiChannel *getPercussionChannel() { return &_midi_channels [9]; }