add mutex/lock to all Evoral::SMF methods that use _smf/libsmf, to avoid inadvertent corruption via multithreaded access.

Serialization of Session::save_state() will already protect against most of this, but there is really no
good reason why Evoral::SMF's API should require single-threaded/explicit serialization.

4 files changed, 37 insertions, 9 deletions

diff --git a/libs/evoral/evoral/SMF.hpp b/libs/evoral/evoral/SMF.hpp
index 5b04e277b9..b851cf6b64 100644
--- a/libs/evoral/evoral/SMF.hpp
+++ b/libs/evoral/evoral/SMF.hpp
@@ -22,6 +22,8 @@
 
 #include <cassert>
 
+#include <glibmm/threads.h>
+
 #include "evoral/visibility.h"
 #include "evoral/types.hpp"
 
@@ -85,6 +87,7 @@ private:
 	smf_t*       _smf;
 	smf_track_t* _smf_track;
 	bool         _empty; ///< true iff file contains(non-empty) events
+	mutable Glib::Threads::Mutex _smf_lock;
 };
 
 }; /* namespace Evoral */
diff --git a/libs/evoral/src/SMF.cpp b/libs/evoral/src/SMF.cpp
index dc3512a0f6..ed9f727bfd 100644
--- a/libs/evoral/src/SMF.cpp
+++ b/libs/evoral/src/SMF.cpp
@@ -37,22 +37,20 @@ namespace Evoral {
 
 SMF::~SMF()
 {
-	if (_smf) {
-		smf_delete(_smf);
-		_smf = 0;
-		_smf_track = 0;
-	}
+	close ();
 }
 
 uint16_t
 SMF::num_tracks() const
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
 	return _smf->number_of_tracks;
 }
 
 uint16_t
 SMF::ppqn() const
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
 	return _smf->ppqn;
 }
 
@@ -62,6 +60,7 @@ SMF::ppqn() const
 int
 SMF::seek_to_track(int track)
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
 	_smf_track = smf_get_track_by_number(_smf, track);
 	if (_smf_track != NULL) {
 		_smf_track->next_event_number = (_smf_track->number_of_events == 0) ? 0 : 1;
@@ -102,6 +101,8 @@ SMF::test(const std::string& path)
 int
 SMF::open(const std::string& path, int track) THROW_FILE_ERROR
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	assert(track >= 1);
 	if (_smf) {
 		smf_delete(_smf);
@@ -115,6 +116,8 @@ SMF::open(const std::string& path, int track) THROW_FILE_ERROR
 		return -1;
 	}
 
+	cerr << "OPen " << _file_path << " for I/O\n";
+
 	if ((_smf = smf_load (f)) == 0) {
 		return -1;
 	}
@@ -145,6 +148,8 @@ SMF::open(const std::string& path, int track) THROW_FILE_ERROR
 int
 SMF::create(const std::string& path, int track, uint16_t ppqn) THROW_FILE_ERROR
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	assert(track >= 1);
 	if (_smf) {
 		smf_delete(_smf);
@@ -196,6 +201,8 @@ SMF::create(const std::string& path, int track, uint16_t ppqn) THROW_FILE_ERROR
 void
 SMF::close() THROW_FILE_ERROR
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	if (_smf) {
 		smf_delete(_smf);
 		_smf = 0;
@@ -206,6 +213,7 @@ SMF::close() THROW_FILE_ERROR
 void
 SMF::seek_to_start() const
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
 	_smf_track->next_event_number = 1;
 }
 
@@ -229,6 +237,8 @@ SMF::seek_to_start() const
 int
 SMF::read_event(uint32_t* delta_t, uint32_t* size, uint8_t** buf, event_id_t* note_id) const
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	smf_event_t* event;
 
 	assert(delta_t);
@@ -291,6 +301,8 @@ SMF::read_event(uint32_t* delta_t, uint32_t* size, uint8_t** buf, event_id_t* no
 void
 SMF::append_event_delta(uint32_t delta_t, uint32_t size, const uint8_t* buf, event_id_t note_id)
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	if (size == 0) {
 		return;
 	}
@@ -364,6 +376,8 @@ SMF::append_event_delta(uint32_t delta_t, uint32_t size, const uint8_t* buf, eve
 void
 SMF::begin_write()
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
+
 	assert(_smf_track);
 	smf_track_delete(_smf_track);
 
@@ -377,6 +391,7 @@ SMF::begin_write()
 void
 SMF::end_write() THROW_FILE_ERROR
 {
+	Glib::Threads::Mutex::Lock lm (_smf_lock);
 	PBD::StdioFileDescriptor d (_file_path, "w+");
 	FILE* f = d.allocate ();
 	if (f == 0) {
diff --git a/libs/evoral/src/libsmf/smf_load.c b/libs/evoral/src/libsmf/smf_load.c
index d8168d0e6a..26ab0e6a1f 100644
--- a/libs/evoral/src/libsmf/smf_load.c
+++ b/libs/evoral/src/libsmf/smf_load.c
@@ -565,11 +565,17 @@ parse_next_event(smf_track_t *track)
 	assert(track->next_event_offset > 0);
 
 	buffer_length = track->file_buffer_length - track->next_event_offset;
-	assert(buffer_length > 0);
-
+	/* if there was no meta-EOT event, buffer_length can be zero. This is
+	   an error in the SMF file, but it shouldn't be treated as fatal.
+	*/
+	if (buffer_length == 0) {
+		g_warning ("SMF warning: expected EOT at end of track, but none found");
+		goto error;
+	}
 	/* First, extract time offset from previous event. */
-	if (smf_extract_vlq(c, buffer_length, &etime, &len))
+	if (smf_extract_vlq(c, buffer_length, &etime, &len)) {
 		goto error;
+	}
 
 	c += len;
 	buffer_length -= len;
@@ -578,8 +584,9 @@ parse_next_event(smf_track_t *track)
 		goto error;
 
 	/* Now, extract the actual event. */
-	if (extract_midi_event(c, buffer_length, event, &len, track->last_status))
+	if (extract_midi_event(c, buffer_length, event, &len, track->last_status)) {
 		goto error;
+	}
 
 	c += len;
 	buffer_length -= len;
diff --git a/libs/evoral/src/libsmf/smf_save.c b/libs/evoral/src/libsmf/smf_save.c
index 120c3a95eb..02cb2084bb 100644
--- a/libs/evoral/src/libsmf/smf_save.c
+++ b/libs/evoral/src/libsmf/smf_save.c
@@ -441,6 +441,9 @@ pointers_are_clear(smf_t *smf)
 	int i;
 
 	smf_track_t *track;
+	if (smf->file_buffer != NULL) {
+		fprintf (stderr, "SFB != null but == %p\n", smf->file_buffer);
+	}
 	assert(smf->file_buffer == NULL);
 	assert(smf->file_buffer_length == 0);