Fix playlist use-count 2/2

* Use an atomic reference count since the freeze-thread
  can call use_playlist.
* Remove explicit argument to construct unused playlist
  because playlists are unused by default. This also
  lead to use-count becoming negative (or rather UINT32_MAX)

libs/ardour/ardour/playlist.h

@@ -143,7 +143,7 @@ public:
 	void release ();
 
 	bool            empty () const;
-	bool            used () const          { return _refcnt != 0; }
+	bool            used () const          { return _refcnt.load () != 0; }
 	int             sort_id () const       { return _sort_id; }
 	bool            frozen () const        { return _frozen; }
 	const DataType& data_type () const     { return _type; }
@@ -377,7 +377,7 @@ protected:
 	bool               _rippling;
 	bool               _shuffling;
 	bool               _nudging;
-	uint32_t           _refcnt;
+	std::atomic<int>   _refcnt;
 	bool               in_flush;
 	bool               in_partition;
 	bool               _frozen;

libs/ardour/ardour/playlist_factory.h

@@ -32,9 +32,9 @@ class Session;
 class LIBARDOUR_API PlaylistFactory {
 
   public:
-	static PBD::Signal2<void,std::shared_ptr<Playlist>, bool> PlaylistCreated;
+	static PBD::Signal1<void,std::shared_ptr<Playlist>> PlaylistCreated;
 
-	static std::shared_ptr<Playlist> create (Session&, const XMLNode&, bool hidden = false, bool unused = false);
+	static std::shared_ptr<Playlist> create (Session&, const XMLNode&, bool hidden = false);
 	static std::shared_ptr<Playlist> create (DataType type, Session&, std::string name, bool hidden = false);
 	static std::shared_ptr<Playlist> create (std::shared_ptr<const Playlist>, std::string name, bool hidden = false);
 	static std::shared_ptr<Playlist> create (std::shared_ptr<const Playlist>, timepos_t const & start, timepos_t const & cnt, std::string name, bool hidden = false);

libs/ardour/ardour/session.h

@@ -902,7 +902,7 @@ public:
 	std::shared_ptr<MidiSource> midi_source_by_path (const std::string&, bool need_source_lock) const;
 	uint32_t count_sources_by_origin (const std::string&);
 
-	void add_playlist (std::shared_ptr<Playlist>, bool unused = false);
+	void add_playlist (std::shared_ptr<Playlist>);
 
 	/* Curves and AutomationLists (TODO when they go away) */
 	void add_automation_list(AutomationList*);

libs/ardour/audio_playlist_importer.cc

@@ -244,7 +244,7 @@ AudioPlaylistImporter::_move ()
 	}
 
 	// Create playlist
-	playlist = PlaylistFactory::create (session, xml_playlist, false, true);
+	playlist = PlaylistFactory::create (session, xml_playlist, false);
 }
 
 void

libs/ardour/audio_track.cc

@@ -449,8 +449,8 @@ void
 AudioTrack::unfreeze ()
 {
 	if (_freeze_record.playlist) {
-		_freeze_record.playlist->release();
 		use_playlist (DataType::AUDIO, _freeze_record.playlist);
+		_freeze_record.playlist->release();
 
 		{
 			Glib::Threads::RWLock::ReaderLock lm (_processor_lock); // should this be a write lock? jlc

libs/ardour/midi_track.cc

@@ -96,6 +96,9 @@ MidiTrack::MidiTrack (Session& sess, string name, TrackMode mode)
 
 MidiTrack::~MidiTrack ()
 {
+	if (_freeze_record.playlist && !_session.deletion_in_progress()) {
+		_freeze_record.playlist->release();
+	}
 }
 
 int

libs/ardour/playlist.cc

@@ -287,18 +287,23 @@ Playlist::Playlist (std::shared_ptr<const Playlist> other, timepos_t const & sta
 void
 Playlist::use ()
 {
-	++_refcnt;
-	InUse (true); /* EMIT SIGNAL */
+	if (0 == _refcnt.fetch_add (1)) {
+		InUse (true); /* EMIT SIGNAL */
+	}
 }
 
 void
 Playlist::release ()
 {
-	if (_refcnt > 0) {
-		_refcnt--;
+	int oldval = _refcnt.fetch_sub (1);
+#ifndef NDEBUG
+	if (oldval <= 0) {
+		cerr << "Bad Playlist::release for " << name() << endl;
 	}
+	assert (oldval > 0);
+#endif
 
-	if (_refcnt == 0) {
+	if (oldval == 1) {
 		InUse (false); /* EMIT SIGNAL */
 	}
 }
@@ -323,7 +328,6 @@ Playlist::init (bool hide)
 	pending_contents_change     = false;
 	pending_layering            = false;
 	first_set_state             = true;
-	_refcnt                     = 0;
 	_hidden                     = hide;
 	_rippling                   = false;
 	_shuffling                  = false;
@@ -335,7 +339,10 @@ Playlist::init (bool hide)
 	subcnt                      = 0;
 	_frozen                     = false;
 	_capture_insertion_underway = false;
-	_combine_ops = 0;
+	_combine_ops                = 0;
+
+	_refcnt.store (0);
+
 	_end_space = timecnt_t (_type == DataType::AUDIO ? Temporal::AudioTime : Temporal::BeatTime);
 	_playlist_shift_active = false;
 

libs/ardour/playlist_factory.cc

@@ -32,10 +32,10 @@ using namespace std;
 using namespace ARDOUR;
 using namespace PBD;
 
-PBD::Signal2<void,std::shared_ptr<Playlist>, bool> PlaylistFactory::PlaylistCreated;
+PBD::Signal1<void,std::shared_ptr<Playlist>> PlaylistFactory::PlaylistCreated;
 
 std::shared_ptr<Playlist>
-PlaylistFactory::create (Session& s, const XMLNode& node, bool hidden, bool unused)
+PlaylistFactory::create (Session& s, const XMLNode& node, bool hidden)
 {
 	XMLProperty const * type = node.property("type");
 
@@ -51,7 +51,7 @@ PlaylistFactory::create (Session& s, const XMLNode& node, bool hidden, bool unus
 		pl->set_region_ownership ();
 
 		if (pl && !hidden) {
-			PlaylistCreated (pl, unused);
+			PlaylistCreated (pl);
 		}
 		return pl;
 
@@ -72,7 +72,7 @@ PlaylistFactory::create (DataType type, Session& s, string name, bool hidden)
 			pl = std::shared_ptr<Playlist> (new MidiPlaylist (s, name, hidden));
 
 		if (pl && !hidden) {
-			PlaylistCreated (pl, false);
+			PlaylistCreated (pl);
 		}
 
 		return pl;
@@ -99,7 +99,7 @@ PlaylistFactory::create (std::shared_ptr<const Playlist> old, string name, bool
 		}
 
 		if (pl && !hidden) {
-			PlaylistCreated (pl, false);
+			PlaylistCreated (pl);
 		}
 
 		return pl;

libs/ardour/playlist_source.cc

@@ -119,9 +119,13 @@ PlaylistSource::set_state (const XMLNode& node, int /*version*/)
 
 	nlist = node.children();
 
+	if (_playlist) {
+		_playlist->release ();
+	}
+
 	for (niter = nlist.begin(); niter != nlist.end(); ++niter) {
 		if ((*niter)->name() == "Playlist") {
-			_playlist = PlaylistFactory::create (_session, **niter, true, false);
+			_playlist = PlaylistFactory::create (_session, **niter, true);
 			break;
 		}
 	}
@@ -129,6 +133,8 @@ PlaylistSource::set_state (const XMLNode& node, int /*version*/)
 	if (!_playlist) {
 		error << _("Could not construct playlist for PlaylistSource from session data!") << endmsg;
 		throw failed_constructor ();
+	} else {
+		_playlist->use ();
 	}
 
 	/* other properties */

libs/ardour/session.cc

@@ -5151,7 +5151,7 @@ Session::playlist_is_active (std::shared_ptr<Playlist> playlist)
 }
 
 void
-Session::add_playlist (std::shared_ptr<Playlist> playlist, bool unused)
+Session::add_playlist (std::shared_ptr<Playlist> playlist)
 {
 	if (playlist->hidden()) {
 		return;
@@ -5159,10 +5159,6 @@ Session::add_playlist (std::shared_ptr<Playlist> playlist, bool unused)
 
 	_playlists->add (playlist);
 
-	if (unused) {
-		playlist->release();
-	}
-
 	set_dirty();
 }
 

libs/ardour/session_playlists.cc

@@ -78,7 +78,11 @@ SessionPlaylists::add (std::shared_ptr<Playlist> playlist)
 	bool const existing = find (playlists.begin(), playlists.end(), playlist) != playlists.end();
 
 	if (!existing) {
-		playlists.insert (playlists.begin(), playlist);
+		if (playlist->used ()) {
+			playlists.insert (playlists.begin(), playlist);
+		} else {
+			unused_playlists.insert (unused_playlists.begin(), playlist);
+		}
 		playlist->InUse.connect_same_thread (*this, boost::bind (&SessionPlaylists::track, this, _1, std::weak_ptr<Playlist>(playlist)));
 		playlist->DropReferences.connect_same_thread (
 			*this, boost::bind (&SessionPlaylists::remove_weak, this, std::weak_ptr<Playlist> (playlist))

libs/ardour/session_state.cc

@@ -210,7 +210,7 @@ Session::pre_engine_init (string fullpath)
 	/* These are all static "per-class" signals */
 
 	SourceFactory::SourceCreated.connect_same_thread (*this, boost::bind (&Session::add_source, this, _1));
-	PlaylistFactory::PlaylistCreated.connect_same_thread (*this, boost::bind (&Session::add_playlist, this, _1, _2));
+	PlaylistFactory::PlaylistCreated.connect_same_thread (*this, boost::bind (&Session::add_playlist, this, _1));
 	AutomationList::AutomationListCreated.connect_same_thread (*this, boost::bind (&Session::add_automation_list, this, _1));
 	IO::PortCountChanged.connect_same_thread (*this, boost::bind (&Session::ensure_buffers, this, _1));
 

libs/ardour/source_factory.cc

@@ -417,7 +417,8 @@ SourceFactory::createFromPlaylist (DataType type, Session& s, std::shared_ptr<Pl
 					start = timecnt_t::zero (Temporal::AudioTime);
 				}
 
-				Source*                   src = new AudioPlaylistSource (s, orig, name, ap, chn, start, len, Source::Flag (0));
+				Source* src = new AudioPlaylistSource (s, orig, name, ap, chn, start, len, Source::Flag (0));
+
 				std::shared_ptr<Source> ret (src);
 
 				if (setup_peakfile (ret, defer_peaks)) {