Prevented potential crashes of mt32emu-qt when a MIDI message is pushed

to SynthRoute while the audioStream is being stopped

mt32emu_qt/src/RealtimeReadLocker.h

@@ -0,0 +1,43 @@
+/* Copyright (C) 2011-2020 Jerome Fisher, Sergey V. Mikayev
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef REALTIME_READ_LOCKER_H
+#define REALTIME_READ_LOCKER_H
+
+#include "QReadWriteLock"
+
+// Similar to QReadLocker but intended to be used from a realtime thread that should never block.
+class RealtimeReadLocker {
+private:
+	QReadWriteLock &lock;
+	bool locked;
+
+public:
+	explicit RealtimeReadLocker(QReadWriteLock &useLock) : lock(useLock) {
+		locked = lock.tryLockForRead();
+	}
+
+	~RealtimeReadLocker() {
+		if (locked) lock.unlock();
+		locked = false;
+	}
+
+	bool isLocked() {
+		return locked;
+	}
+};
+
+#endif // REALTIME_READ_LOCKER_H

mt32emu_qt/src/SynthRoute.cpp

@@ -29,6 +29,7 @@
 #include "SynthRoute.h"
 #include "MidiSession.h"
 #include "QMidiBuffer.h"
+#include "RealtimeReadLocker.h"
 #include "audiodrv/AudioDriver.h"
 
 using namespace MT32Emu;
@@ -47,7 +48,7 @@ SynthRoute::SynthRoute(QObject *parent) :
 }
 
 SynthRoute::~SynthRoute() {
-	delete audioStream;
+	deleteAudioStream();
 }
 
 void SynthRoute::setAudioDevice(const AudioDevice *newAudioDevice) {
@@ -83,13 +84,13 @@ bool SynthRoute::open(AudioStreamFactory audioStreamFactory) {
 			debugDeltaUpperLimit = qint64(ceil(debugDeltaMean + debugDeltaLimit));
 			qDebug() << "Using sample rate:" << sampleRate;
 
-			if (exclusiveMidiMode && audioStreamFactory != NULL) {
-				audioStream = audioStreamFactory(audioDevice, *this, sampleRate, midiSessions.first());
-			} else {
-				audioStream = audioDevice->startAudioStream(*this, sampleRate);
-			}
-			if (audioStream != NULL) {
+			AudioStream *newAudioStream = exclusiveMidiMode && audioStreamFactory != NULL
+				? audioStreamFactory(audioDevice, *this, sampleRate, midiSessions.first())
+				: audioDevice->startAudioStream(*this, sampleRate);
+			if (newAudioStream != NULL) {
 				setState(SynthRouteState_OPEN);
+				QWriteLocker audioStreamLocker(&audioStreamLock);
+				audioStream = newAudioStream;
 				return true;
 			} else {
 				qDebug() << "Failed to start audioStream";
@@ -116,8 +117,7 @@ bool SynthRoute::close() {
 		break;
 	}
 	setState(SynthRouteState_CLOSING);
-	delete audioStream;
-	audioStream = NULL;
+	deleteAudioStream();
 	qSynth.close();
 	disableExclusiveMidiMode();
 	discardMidiBuffers();
@@ -201,8 +201,7 @@ void SynthRoute::handleQSynthState(SynthState synthState) {
 		setState(SynthRouteState_CLOSING);
 		break;
 	case SynthState_CLOSED:
-		delete audioStream;
-		audioStream = NULL;
+		deleteAudioStream();
 		setState(SynthRouteState_CLOSED);
 		disableExclusiveMidiMode();
 		break;
@@ -219,9 +218,12 @@ bool SynthRoute::connectReportHandler(const char *signal, const QObject *receive
 
 bool SynthRoute::pushMIDIShortMessage(MidiSession &midiSession, Bit32u msg, MasterClockNanos refNanos) {
 	if (midiRecorder.isRecording()) midiSession.getMidiTrackRecorder()->recordShortMessage(msg, refNanos);
-	AudioStream *stream = audioStream;
-	if (stream == NULL) return false;
-	quint64 timestamp = stream->estimateMIDITimestamp(refNanos);
+	quint64 timestamp;
+	{
+		RealtimeReadLocker audioStreamLocker(audioStreamLock);
+		if (!audioStreamLocker.isLocked() || audioStream == NULL) return false;
+		timestamp = audioStream->estimateMIDITimestamp(refNanos);
+	}
 	if (msg == 0) {
 		// This is a special event sent by the test driver
 		qint64 delta = qint64(timestamp) - qint64(debugLastEventTimestamp);
@@ -237,9 +239,12 @@ bool SynthRoute::pushMIDIShortMessage(MidiSession &midiSession, Bit32u msg, Mast
 
 bool SynthRoute::pushMIDISysex(MidiSession &midiSession, const Bit8u *sysexData, unsigned int sysexLen, MasterClockNanos refNanos) {
 	if (midiRecorder.isRecording()) midiSession.getMidiTrackRecorder()->recordSysex(sysexData, sysexLen, refNanos);
-	AudioStream *stream = audioStream;
-	if (stream == NULL) return false;
-	quint64 timestamp = stream->estimateMIDITimestamp(refNanos);
+	quint64 timestamp;
+	{
+		RealtimeReadLocker audioStreamLocker(audioStreamLock);
+		if (!audioStreamLocker.isLocked() || audioStream == NULL) return false;
+		timestamp = audioStream->estimateMIDITimestamp(refNanos);
+	}
 	return playMIDISysex(midiSession, sysexData, sysexLen, timestamp);
 }
 
@@ -291,12 +296,20 @@ void SynthRoute::flushMIDIQueue() {
 
 // When renderingPassFrameLength == 0, all pending messages are merged.
 void SynthRoute::mergeMidiStreams(uint renderingPassFrameLength) {
+	quint64 renderingPassEndTimestamp;
+	{
+		if (renderingPassFrameLength > 0) {
+			RealtimeReadLocker audioStreamLocker(audioStreamLock);
+			// Occasionally, audioStream may appear NULL during startup.
+			if (!audioStreamLocker.isLocked() || audioStream == NULL) return;
+			renderingPassEndTimestamp = audioStream->computeMIDITimestamp(renderingPassFrameLength);
+		} else {
+			renderingPassEndTimestamp = std::numeric_limits<quint64>::max();
+		}
+	}
+
 	QMutexLocker midiSessionsLocker(&midiSessionsMutex);
 	QVarLengthArray<QMidiBuffer *, 16> streamBuffers;
-	const quint64 renderingPassEndTimestamp = renderingPassFrameLength == 0
-		? std::numeric_limits<quint64>::max()
-		: audioStream->computeMIDITimestamp(renderingPassFrameLength);
-
 	for (int i = 0; i < midiSessions.size(); i++) {
 		QMidiBuffer *midiBuffer = midiSessions[i]->getQMidiBuffer();
 		if (midiBuffer->retieveEvents() && midiBuffer->getEventTimestamp() < renderingPassEndTimestamp) {
@@ -338,15 +351,19 @@ void SynthRoute::mergeMidiStreams(uint renderingPassFrameLength) {
 	}
 }
 
+void SynthRoute::deleteAudioStream() {
+	QWriteLocker audioStreamLocker(&audioStreamLock);
+	delete audioStream;
+	audioStream = NULL;
+}
+
 void SynthRoute::render(MT32Emu::Bit16s *buffer, uint length) {
-	// Occasionally, audioStream may appear NULL during startup.
-	if (multiMidiMode && audioStream != NULL) mergeMidiStreams(length);
+	if (multiMidiMode) mergeMidiStreams(length);
 	qSynth.render(buffer, length);
 }
 
 void SynthRoute::render(float *buffer, uint length) {
-	// Occasionally, audioStream may appear NULL during startup.
-	if (multiMidiMode && audioStream != NULL) mergeMidiStreams(length);
+	if (multiMidiMode) mergeMidiStreams(length);
 	qSynth.render(buffer, length);
 }
 

mt32emu_qt/src/SynthRoute.h

@@ -33,7 +33,10 @@ private:
 	volatile bool multiMidiMode;
 
 	const AudioDevice *audioDevice;
-	AudioStream *audioStream; // NULL until a stream is created
+	// Points to the currently active AudioStream, NULL if there are none.
+	AudioStream *audioStream;
+	// Protects read accesses to audioStream against concurrent deletions.
+	QReadWriteLock audioStreamLock;
 
 	quint64 debugLastEventTimestamp;
 	qint64 debugDeltaLowerLimit, debugDeltaUpperLimit;
@@ -41,6 +44,7 @@ private:
 	void setState(SynthRouteState newState);
 	void disableExclusiveMidiMode();
 	void mergeMidiStreams(uint renderingPassFrameLength);
+	void deleteAudioStream();
 
 public:
 	SynthRoute(QObject *parent = NULL);