remove recursive mutex from Playlist, replace with private regular mutex, force everyone to use Playlist::RegionLock to allow checking on lock handling if necessary; fix recursive use of lock in AudioPlaylist::read()

git-svn-id: svn://localhost/ardour2/branches/3.0@12131 d708f5d6-7413-0410-9779-e7cbd77b26cf

4 files changed, 142 insertions, 150 deletions

diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h
index 559a24062b..9a76ec56ad 100644
--- a/libs/ardour/ardour/playlist.h
+++ b/libs/ardour/ardour/playlist.h
@@ -39,6 +39,7 @@
 #include "pbd/stateful.h"
 #include "pbd/statefuldestructible.h"
 #include "pbd/sequence_property.h"
+#include "pbd/stacktrace.h"
 
 #include "evoral/types.hpp"
 
@@ -231,7 +232,10 @@ public:
   protected:
 	struct RegionLock {
 		RegionLock (Playlist *pl, bool do_block_notify = true) : playlist (pl), block_notify (do_block_notify) {
-			playlist->region_lock.lock();
+			if (!playlist->region_lock.trylock()) {
+				std::cerr << "Lock for playlist " << pl->name() << " already held\n";
+				PBD::stacktrace (std::cerr, 10);
+			}	
 			if (block_notify) {
 				playlist->delay_notifications();
 			}
@@ -246,8 +250,6 @@ public:
 		bool block_notify;
 	};
 
-	friend class RegionLock;
-
 	RegionListProperty   regions;  /* the current list of regions in the playlist */
 	std::set<boost::shared_ptr<Region> > all_regions; /* all regions ever added to this playlist */
 	PBD::ScopedConnectionList region_state_changed_connections;
@@ -255,11 +257,6 @@ public:
 	int             _sort_id;
 	mutable gint    block_notifications;
 	mutable gint    ignore_state_changes;
-#ifdef HAVE_GLIB_THREADS_RECMUTEX
-	mutable Glib::Threads::RecMutex region_lock;
-#else
-	mutable Glib::RecMutex region_lock;
-#endif	
 	std::set<boost::shared_ptr<Region> > pending_adds;
 	std::set<boost::shared_ptr<Region> > pending_removes;
 	RegionList       pending_bounds;
@@ -303,6 +300,8 @@ public:
 
 	void _set_sort_id ();
 
+	boost::shared_ptr<RegionList> regions_touched_locked (framepos_t start, framepos_t end);
+
 	void notify_region_removed (boost::shared_ptr<Region>);
 	void notify_region_added (boost::shared_ptr<Region>);
 	void notify_layering_changed ();
@@ -365,8 +364,11 @@ public:
 	*/
 	virtual void pre_uncombine (std::vector<boost::shared_ptr<Region> >&, boost::shared_ptr<Region>) {}
 
-private:
+  private:
+	friend class RegionLock;
+	mutable Glib::Mutex region_lock;
 
+  private:
 	void setup_layering_indices (RegionList const &);
 	void coalesce_and_check_crossfades (std::list<Evoral::Range<framepos_t> >);
 	boost::shared_ptr<RegionList> find_regions_at (framepos_t);
diff --git a/libs/ardour/audio_playlist.cc b/libs/ardour/audio_playlist.cc
index 9c304967dc..867f6f0ce4 100644
--- a/libs/ardour/audio_playlist.cc
+++ b/libs/ardour/audio_playlist.cc
@@ -186,16 +186,12 @@ AudioPlaylist::read (Sample *buf, Sample *mixdown_buffer, float *gain_buffer, fr
 	   its OK to block (for short intervals).
 	*/
 
-#ifdef HAVE_GLIB_THREADS_RECMUTEX
-	Glib::Threads::RecMutex::Lock lm (region_lock);
-#else	
-	Glib::RecMutex::Lock rm (region_lock);
-#endif	
+	Playlist::RegionLock rl (this, false);
 
 	/* Find all the regions that are involved in the bit we are reading,
 	   and sort them by descending layer and ascending position.
 	*/
-	boost::shared_ptr<RegionList> all = regions_touched (start, start + cnt - 1);
+	boost::shared_ptr<RegionList> all = regions_touched_locked (start, start + cnt - 1);
 	all->sort (ReadSorter ());
 
 	/* This will be a list of the bits of our read range that we have
diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc
index 7e3d70bfe0..81b14dd933 100644
--- a/libs/ardour/midi_playlist.cc
+++ b/libs/ardour/midi_playlist.cc
@@ -107,11 +107,8 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 	   its OK to block (for short intervals).
 	*/
 
-#ifdef HAVE_GLIB_THREADS_RECMUTEX
-	Glib::Threads::RecMutex::Lock rm (region_lock);
-#else	
-	Glib::RecMutex::Lock rm (region_lock);
-#endif	
+	Playlist::RegionLock rl (this, false);
+
 	DEBUG_TRACE (DEBUG::MidiPlaylistIO, string_compose ("++++++ %1 .. %2  +++++++ %3 trackers +++++++++++++++++\n", 
 							    start, start + dur, _note_trackers.size()));
 
@@ -298,11 +295,8 @@ MidiPlaylist::read (Evoral::EventSink<framepos_t>& dst, framepos_t start, framec
 void
 MidiPlaylist::clear_note_trackers ()
 {
-#ifdef HAVE_GLIB_THREADS_RECMUTEX
-	Glib::Threads::RecMutex::Lock rm (region_lock);
-#else	
-	Glib::RecMutex::Lock rm (region_lock);
-#endif	
+	Playlist::RegionLock rl (this, false);
+
 	for (NoteTrackers::iterator n = _note_trackers.begin(); n != _note_trackers.end(); ++n) {
 		delete n->second;
 	}
@@ -407,12 +401,7 @@ MidiPlaylist::contained_automation()
 	   its OK to block (for short intervals).
 	*/
 
-#ifdef HAVE_GLIB_THREADS_RECMUTEX
-	Glib::Threads::RecMutex::Lock rm (region_lock);
-#else	
-	Glib::RecMutex::Lock rm (region_lock);
-#endif	
-
+	Playlist::RegionLock rl (this, false);
 	set<Evoral::Parameter> ret;
 
 	for (RegionList::const_iterator r = regions.begin(); r != regions.end(); ++r) {
diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc
index 076c4f27ca..7be5d418f5 100644
--- a/libs/ardour/playlist.cc
+++ b/libs/ardour/playlist.cc
@@ -1758,6 +1758,12 @@ boost::shared_ptr<RegionList>
 Playlist::regions_touched (framepos_t start, framepos_t end)
 {
 	RegionLock rlock (this);
+	return regions_touched_locked (start, end);
+}
+
+boost::shared_ptr<RegionList>
+Playlist::regions_touched_locked (framepos_t start, framepos_t end)
+{
 	boost::shared_ptr<RegionList> rlist (new RegionList);
 	
 	for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
@@ -1769,126 +1775,125 @@ Playlist::regions_touched (framepos_t start, framepos_t end)
 	return rlist;
 }
 
- framepos_t
- Playlist::find_next_transient (framepos_t from, int dir)
- {
-	 RegionLock rlock (this);
-	 AnalysisFeatureList points;
-	 AnalysisFeatureList these_points;
-
-	 for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
-		 if (dir > 0) {
-			 if ((*i)->last_frame() < from) {
-				 continue;
-			 }
-		 } else {
-			 if ((*i)->first_frame() > from) {
-				 continue;
-			 }
-		 }
-
-		 (*i)->get_transients (these_points);
-
-		 /* add first frame, just, err, because */
-
-		 these_points.push_back ((*i)->first_frame());
-
-		 points.insert (points.end(), these_points.begin(), these_points.end());
-		 these_points.clear ();
-	 }
-
-	 if (points.empty()) {
-		 return -1;
-	 }
-
-	 TransientDetector::cleanup_transients (points, _session.frame_rate(), 3.0);
-	 bool reached = false;
-
-	 if (dir > 0) {
-		 for (AnalysisFeatureList::iterator x = points.begin(); x != points.end(); ++x) {
-			 if ((*x) >= from) {
-				 reached = true;
-			 }
-
-			 if (reached && (*x) > from) {
-				 return *x;
-			 }
-		 }
-	 } else {
-		 for (AnalysisFeatureList::reverse_iterator x = points.rbegin(); x != points.rend(); ++x) {
-			 if ((*x) <= from) {
-				 reached = true;
-			 }
-
-			 if (reached && (*x) < from) {
-				 return *x;
-			 }
-		 }
-	 }
-
-	 return -1;
- }
-
- boost::shared_ptr<Region>
- Playlist::find_next_region (framepos_t frame, RegionPoint point, int dir)
- {
-	 RegionLock rlock (this);
-	 boost::shared_ptr<Region> ret;
-	 framepos_t closest = max_framepos;
-
-	 bool end_iter = false;
-
-	 for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
-
-		 if(end_iter) break;
-
-		 frameoffset_t distance;
-		 boost::shared_ptr<Region> r = (*i);
-		 framepos_t pos = 0;
-
-		 switch (point) {
-		 case Start:
-			 pos = r->first_frame ();
-			 break;
-		 case End:
-			 pos = r->last_frame ();
-			 break;
-		 case SyncPoint:
-			 pos = r->sync_position ();
-			 break;
-		 }
-
-		 switch (dir) {
-		 case 1: /* forwards */
-
-			 if (pos > frame) {
-				 if ((distance = pos - frame) < closest) {
-					 closest = distance;
-					 ret = r;
-					 end_iter = true;
-				 }
-			 }
-
-			 break;
-
-		 default: /* backwards */
-
-			 if (pos < frame) {
-				 if ((distance = frame - pos) < closest) {
-					 closest = distance;
-					 ret = r;
-				 }
-			 }
-			 else {
-				 end_iter = true;
-			 }
-
-			 break;
-		 }
-	 }
+framepos_t
+Playlist::find_next_transient (framepos_t from, int dir)
+{
+	RegionLock rlock (this);
+	AnalysisFeatureList points;
+	AnalysisFeatureList these_points;
+	
+	for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
+		if (dir > 0) {
+			if ((*i)->last_frame() < from) {
+				continue;
+			}
+		} else {
+			if ((*i)->first_frame() > from) {
+				continue;
+			}
+		}
+		
+		(*i)->get_transients (these_points);
+		
+		/* add first frame, just, err, because */
+		
+		these_points.push_back ((*i)->first_frame());
+		
+		points.insert (points.end(), these_points.begin(), these_points.end());
+		these_points.clear ();
+	}
+	
+	if (points.empty()) {
+		return -1;
+	}
+	
+	TransientDetector::cleanup_transients (points, _session.frame_rate(), 3.0);
+	bool reached = false;
+	
+	if (dir > 0) {
+		for (AnalysisFeatureList::iterator x = points.begin(); x != points.end(); ++x) {
+			if ((*x) >= from) {
+				reached = true;
+			}
+			
+			if (reached && (*x) > from) {
+				return *x;
+			}
+		}
+	} else {
+		for (AnalysisFeatureList::reverse_iterator x = points.rbegin(); x != points.rend(); ++x) {
+			if ((*x) <= from) {
+				reached = true;
+			}
+			
+			if (reached && (*x) < from) {
+				return *x;
+			}
+		}
+	}
+	
+	return -1;
+}
 
-	 return ret;
- }
+boost::shared_ptr<Region>
+Playlist::find_next_region (framepos_t frame, RegionPoint point, int dir)
+{
+	RegionLock rlock (this);
+	boost::shared_ptr<Region> ret;
+	framepos_t closest = max_framepos;
+	
+	bool end_iter = false;
+	
+	for (RegionList::iterator i = regions.begin(); i != regions.end(); ++i) {
+		
+		if(end_iter) break;
+		
+		frameoffset_t distance;
+		boost::shared_ptr<Region> r = (*i);
+		framepos_t pos = 0;
+		
+		switch (point) {
+		case Start:
+			pos = r->first_frame ();
+			break;
+		case End:
+			pos = r->last_frame ();
+			break;
+		case SyncPoint:
+			pos = r->sync_position ();
+			break;
+		}
+		
+		switch (dir) {
+		case 1: /* forwards */
+			
+			if (pos > frame) {
+				if ((distance = pos - frame) < closest) {
+					closest = distance;
+					ret = r;
+					end_iter = true;
+				}
+			}
+			
+			break;
+			
+		default: /* backwards */
+			
+			if (pos < frame) {
+				if ((distance = frame - pos) < closest) {
+					closest = distance;
+					ret = r;
+				}
+			} else {
+				end_iter = true;
+			}
+			
+			break;
+		}
+	}
+	
+	return ret;
+}
 
  framepos_t
  Playlist::find_next_region_boundary (framepos_t frame, int dir)