Fix various MIDI locking issues.

Attempt to make mistakes much less likely in the future by statically requiring
caller to pass scoped locks where necessary.

8 files changed, 78 insertions, 50 deletions

diff --git a/libs/ardour/ardour/audiofilesource.h b/libs/ardour/ardour/audiofilesource.h
index 4831eb2081..9e63f4c81d 100644
--- a/libs/ardour/ardour/audiofilesource.h
+++ b/libs/ardour/ardour/audiofilesource.h
@@ -64,7 +64,7 @@ public:
 	virtual int update_header (framepos_t when, struct tm&, time_t) = 0;
 	virtual int flush_header () = 0;
 
-	void mark_streaming_write_completed ();
+	void mark_streaming_write_completed (const Lock& lock);
 
 	int setup_peakfile ();
 
diff --git a/libs/ardour/ardour/audiosource.h b/libs/ardour/ardour/audiosource.h
index 7d1878f286..622bc6052a 100644
--- a/libs/ardour/ardour/audiosource.h
+++ b/libs/ardour/ardour/audiosource.h
@@ -60,7 +60,7 @@ class LIBARDOUR_API AudioSource : virtual public Source,
 
 	virtual float sample_rate () const = 0;
 
-	virtual void mark_streaming_write_completed ();
+	virtual void mark_streaming_write_completed (const Lock& lock);
 
 	virtual bool can_truncate_peaks() const { return true; }
 
diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h
index 4d12839625..1988c1a2d1 100644
--- a/libs/ardour/ardour/midi_model.h
+++ b/libs/ardour/ardour/midi_model.h
@@ -238,10 +238,15 @@ public:
 	void apply_command (Session& session, Command* cmd);
 	void apply_command_as_subcommand (Session& session, Command* cmd);
 
-	bool sync_to_source ();
-	bool write_to(boost::shared_ptr<MidiSource> source);
-	bool write_section_to (boost::shared_ptr<MidiSource> source, Evoral::MusicalTime begin = Evoral::MinMusicalTime,
-	Evoral::MusicalTime end = Evoral::MaxMusicalTime);
+	bool sync_to_source (const Glib::Threads::Mutex::Lock& source_lock);
+
+	bool write_to(boost::shared_ptr<MidiSource>     source,
+	              const Glib::Threads::Mutex::Lock& source_lock);
+
+	bool write_section_to(boost::shared_ptr<MidiSource>     source,
+	                      const Glib::Threads::Mutex::Lock& source_lock,
+	                      Evoral::MusicalTime               begin = Evoral::MinMusicalTime,
+	                      Evoral::MusicalTime               end   = Evoral::MaxMusicalTime);
 
 	// MidiModel doesn't use the normal AutomationList serialisation code
 	// since controller data is stored in the .mid
diff --git a/libs/ardour/ardour/midi_playlist_source.h b/libs/ardour/ardour/midi_playlist_source.h
index 7a61f5aa02..f064553342 100644
--- a/libs/ardour/ardour/midi_playlist_source.h
+++ b/libs/ardour/ardour/midi_playlist_source.h
@@ -45,10 +45,10 @@ public:
 	XMLNode& get_state ();
 	int set_state (const XMLNode&, int version);
 
-	void append_event_unlocked_beats(const Evoral::Event<Evoral::MusicalTime>& ev);
-	void append_event_unlocked_frames(const Evoral::Event<framepos_t>& ev, framepos_t source_start);
-	void load_model(bool lock=true, bool force_reload=false);
-	void destroy_model();
+	void append_event_beats(const Glib::Threads::Mutex::Lock& lock, const Evoral::Event<Evoral::MusicalTime>& ev);
+	void append_event_frames(const Glib::Threads::Mutex::Lock& lock, const Evoral::Event<framepos_t>& ev, framepos_t source_start);
+	void load_model(const Glib::Threads::Mutex::Lock& lock, bool force_reload=false);
+	void destroy_model(const Glib::Threads::Mutex::Lock& lock);
 
 protected:
 	friend class SourceFactory;
@@ -58,15 +58,17 @@ protected:
 	MidiPlaylistSource (Session&, const XMLNode&);
 
 
-	void flush_midi();
+	void flush_midi(const Lock& lock);
 
-	framecnt_t read_unlocked (Evoral::EventSink<framepos_t>& dst,
+	framecnt_t read_unlocked (const Lock&                    lock,
+	                          Evoral::EventSink<framepos_t>& dst,
 	                          framepos_t                     position,
 	                          framepos_t                     start,
 	                          framecnt_t                     cnt,
 	                          MidiStateTracker*              tracker) const;
 
-	framecnt_t write_unlocked (MidiRingBuffer<framepos_t>& dst,
+	framecnt_t write_unlocked (const Lock&                 lock,
+	                           MidiRingBuffer<framepos_t>& dst,
 	                           framepos_t                  position,
 	                           framecnt_t                  cnt);
 
diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h
index 2ce92ba3cf..2b78230a00 100644
--- a/libs/ardour/ardour/midi_source.h
+++ b/libs/ardour/ardour/midi_source.h
@@ -57,7 +57,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 * \param end time of latest event that can be written.
 	 * \return zero on success, non-zero if the write failed for any reason.
 	 */
-	int write_to (boost::shared_ptr<MidiSource> newsrc,
+	int write_to (const Lock&                   lock,
+	              boost::shared_ptr<MidiSource> newsrc,
 	              Evoral::MusicalTime           begin = Evoral::MinMusicalTime,
 	              Evoral::MusicalTime           end   = Evoral::MaxMusicalTime);
 
@@ -70,7 +71,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 * \param tracker an optional pointer to MidiStateTracker object, for note on/off tracking.
 	 * \param filtered Parameters whose MIDI messages will not be returned.
 	 */
-	virtual framecnt_t midi_read (Evoral::EventSink<framepos_t>&     dst,
+	virtual framecnt_t midi_read (const Lock&                        lock,
+	                              Evoral::EventSink<framepos_t>&     dst,
 	                              framepos_t                         source_start,
 	                              framepos_t                         start,
 	                              framecnt_t                         cnt,
@@ -82,22 +84,33 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 *  @param source_start This source's start position in session frames.
 	 *  @param cnt The length of time to write.
 	 */
-	virtual framecnt_t midi_write (MidiRingBuffer<framepos_t>& src,
+	virtual framecnt_t midi_write (const Lock&                 lock,
+	                               MidiRingBuffer<framepos_t>& src,
 	                               framepos_t                  source_start,
 	                               framecnt_t                  cnt);
 
-	virtual void append_event_unlocked_beats(const Evoral::Event<Evoral::MusicalTime>& ev) = 0;
+	/** Append a single event with a timestamp in beats.
+	 *
+	 * Caller must ensure that the event is later than the last written event.
+	 */
+	virtual void append_event_beats(const Lock&                               lock,
+	                                const Evoral::Event<Evoral::MusicalTime>& ev) = 0;
 
-	virtual void append_event_unlocked_frames(const Evoral::Event<framepos_t>& ev,
-	                                          framepos_t                       source_start) = 0;
+	/** Append a single event with a timestamp in frames.
+	 *
+	 * Caller must ensure that the event is later than the last written event.
+	 */
+	virtual void append_event_frames(const Lock&                      lock,
+	                                 const Evoral::Event<framepos_t>& ev,
+	                                 framepos_t                       source_start) = 0;
 
 	virtual bool       empty () const;
 	virtual framecnt_t length (framepos_t pos) const;
 	virtual void       update_length (framecnt_t);
 
-	virtual void mark_streaming_midi_write_started (NoteMode mode);
-	virtual void mark_streaming_write_started ();
-	virtual void mark_streaming_write_completed ();
+	virtual void mark_streaming_midi_write_started (const Lock& lock, NoteMode mode);
+	virtual void mark_streaming_write_started (const Lock& lock);
+	virtual void mark_streaming_write_completed (const Lock& lock);
 
 	/** Mark write starting with the given time parameters.
 	 *
@@ -119,6 +132,7 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 * etc.
 	 */
 	virtual void mark_midi_streaming_write_completed (
+		const Lock&                                            lock,
 		Evoral::Sequence<Evoral::MusicalTime>::StuckNoteOption stuck_option,
 		Evoral::MusicalTime                                    when = Evoral::MusicalTime());
 
@@ -137,19 +151,17 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	void     set_length_beats(TimeType l) { _length_beats = l; }
 	TimeType length_beats() const         { return _length_beats; }
 
-	virtual void load_model(bool lock=true, bool force_reload=false) = 0;
-	virtual void destroy_model() = 0;
+	virtual void load_model(const Glib::Threads::Mutex::Lock& lock, bool force_reload=false) = 0;
+	virtual void destroy_model(const Glib::Threads::Mutex::Lock& lock) = 0;
 
-	/** This must be called with the source lock held whenever the
-	 *  source/model contents have been changed (reset iterators/cache/etc).
-	 */
-	void invalidate();
+	/** Reset cached information (like iterators) when things have changed. */
+	void invalidate(const Glib::Threads::Mutex::Lock& lock);
 
-	void set_note_mode(NoteMode mode);
+	void set_note_mode(const Glib::Threads::Mutex::Lock& lock, NoteMode mode);
 
 	boost::shared_ptr<MidiModel> model() { return _model; }
-	void set_model (boost::shared_ptr<MidiModel>);
-	void drop_model();
+	void set_model(const Glib::Threads::Mutex::Lock& lock, boost::shared_ptr<MidiModel>);
+	void drop_model(const Glib::Threads::Mutex::Lock& lock);
 
 	Evoral::ControlList::InterpolationStyle interpolation_of (Evoral::Parameter) const;
 	void set_interpolation_of (Evoral::Parameter, Evoral::ControlList::InterpolationStyle);
@@ -169,9 +181,10 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	PBD::Signal2<void, Evoral::Parameter, AutoState> AutomationStateChanged;
 
   protected:
-	virtual void flush_midi() = 0;
+	virtual void flush_midi(const Lock& lock) = 0;
 
-	virtual framecnt_t read_unlocked (Evoral::EventSink<framepos_t>& dst,
+	virtual framecnt_t read_unlocked (const Lock&                    lock,
+	                                  Evoral::EventSink<framepos_t>& dst,
 	                                  framepos_t                     position,
 	                                  framepos_t                     start,
 	                                  framecnt_t                     cnt,
@@ -182,7 +195,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha
 	 *  @param position This source's start position in session frames.
 	 *  @param cnt The duration of this block to write for.
 	 */
-	virtual framecnt_t write_unlocked (MidiRingBuffer<framepos_t>& source,
+	virtual framecnt_t write_unlocked (const Lock&                 lock,
+	                                   MidiRingBuffer<framepos_t>& source,
 	                                   framepos_t                  position,
 	                                   framecnt_t                  cnt) = 0;
 
diff --git a/libs/ardour/ardour/midi_state_tracker.h b/libs/ardour/ardour/midi_state_tracker.h
index d8a31c10aa..98611bc722 100644
--- a/libs/ardour/ardour/midi_state_tracker.h
+++ b/libs/ardour/ardour/midi_state_tracker.h
@@ -20,6 +20,8 @@
 #ifndef __ardour_midi_state_tracker_h__
 #define __ardour_midi_state_tracker_h__
 
+#include <glibmm/threads.h>
+
 #include "ardour/midi_buffer.h"
 
 namespace Evoral {
@@ -44,7 +46,7 @@ public:
 	void remove (uint8_t note, uint8_t chn);
 	void resolve_notes (MidiBuffer& buffer, framepos_t time);
 	void resolve_notes (Evoral::EventSink<framepos_t>& buffer, framepos_t time);
-	void resolve_notes (MidiSource& src, Evoral::MusicalTime time);
+	void resolve_notes (MidiSource& src, const Glib::Threads::Mutex::Lock& lock, Evoral::MusicalTime time);
 	void dump (std::ostream&);
 	void reset ();
 	bool empty() const { return _on == 0; }
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index 56b70b15bd..d93fdb8506 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -47,26 +47,24 @@ public:
 
 	virtual ~SMFSource ();
 
-        bool safe_file_extension (const std::string& path) const {
+	bool safe_file_extension (const std::string& path) const {
 		return safe_midi_file_extension(path);
 	}
 
-	void append_event_unlocked_beats (const Evoral::Event<Evoral::MusicalTime>& ev);
-	void append_event_unlocked_frames (const Evoral::Event<framepos_t>& ev, framepos_t source_start);
+	void append_event_beats (const Lock& lock, const Evoral::Event<Evoral::MusicalTime>& ev);
+	void append_event_frames (const Lock& lock, const Evoral::Event<framepos_t>& ev, framepos_t source_start);
 
-	void mark_streaming_midi_write_started (NoteMode mode);
-	void mark_streaming_write_completed ();
-	void mark_midi_streaming_write_completed (Evoral::Sequence<Evoral::MusicalTime>::StuckNoteOption,
+	void mark_streaming_midi_write_started (const Lock& lock, NoteMode mode);
+	void mark_streaming_write_completed (const Lock& lock);
+	void mark_midi_streaming_write_completed (const Lock& lock,
+	                                          Evoral::Sequence<Evoral::MusicalTime>::StuckNoteOption,
 	                                          Evoral::MusicalTime when = Evoral::MusicalTime());
 
 	XMLNode& get_state ();
 	int set_state (const XMLNode&, int version);
 
-	void load_model (bool lock=true, bool force_reload=false);
-	void destroy_model ();
-
-	void flush_midi ();
-	void ensure_disk_file ();
+	void load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload=false);
+	void destroy_model (const Glib::Threads::Mutex::Lock& lock);
 
 	static bool safe_midi_file_extension (const std::string& path);
 	static bool valid_midi_file (const std::string& path);
@@ -75,6 +73,7 @@ public:
 
   protected:
 	void set_path (const std::string& newpath);
+	void flush_midi (const Lock& lock);
 
   private:
 	bool _open;
@@ -87,13 +86,17 @@ public:
 
 	int open_for_write ();
 	
-	framecnt_t read_unlocked (Evoral::EventSink<framepos_t>& dst,
+	void ensure_disk_file (const Lock& lock);
+
+	framecnt_t read_unlocked (const Lock&                    lock,
+	                          Evoral::EventSink<framepos_t>& dst,
 	                          framepos_t                     position,
 	                          framepos_t                     start,
 	                          framecnt_t                     cnt,
 	                          MidiStateTracker*              tracker) const;
 
-	framecnt_t write_unlocked (MidiRingBuffer<framepos_t>& src,
+	framecnt_t write_unlocked (const Lock&                 lock,
+	                           MidiRingBuffer<framepos_t>& src,
 	                           framepos_t                  position,
 	                           framecnt_t                  cnt);
 
diff --git a/libs/ardour/ardour/source.h b/libs/ardour/ardour/source.h
index e009e0ef51..18de40771c 100644
--- a/libs/ardour/ardour/source.h
+++ b/libs/ardour/ardour/source.h
@@ -51,6 +51,8 @@ class LIBARDOUR_API Source : public SessionObject
 		Empty = 0x100, /* used for MIDI only */
 	};
 
+	typedef Glib::Threads::Mutex::Lock Lock;
+
 	Source (Session&, DataType type, const std::string& name, Flag flags=Flag(0));
 	Source (Session&, const XMLNode&);
 
@@ -69,8 +71,8 @@ class LIBARDOUR_API Source : public SessionObject
 
 	void mark_for_remove();
 
-	virtual void mark_streaming_write_started () {}
-	virtual void mark_streaming_write_completed () = 0;
+	virtual void mark_streaming_write_started (const Lock& lock) {}
+	virtual void mark_streaming_write_completed (const Lock& lock) = 0;
 
 	virtual void session_saved() {}