changes in logic used by source cleanup to avoid endless recursion in sessions with deeply nested/recursive compound regions.

This also fixes some potentially dangerous cleanup logic related to two sources with the same name (but different paths)

libs/ardour/ardour/playlist.h

@@ -130,7 +130,6 @@ public:
 
 	void add_region (boost::shared_ptr<Region>, framepos_t position, float times = 1, bool auto_partition = false);
 	void remove_region (boost::shared_ptr<Region>);
-	void remove_region_by_source (boost::shared_ptr<Source>);
 	void get_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
 	void get_region_list_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
 	void get_source_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
@@ -181,7 +180,8 @@ public:
 	bool                       region_is_shuffle_constrained (boost::shared_ptr<Region>);
 	bool                       has_region_at (framepos_t const) const;
 
-	bool uses_source (boost::shared_ptr<const Source> src) const;
+	bool uses_source (boost::shared_ptr<const Source> src, bool shallow = false) const;
+	void deep_sources (std::set<boost::shared_ptr<Source> >&) const;
 
 	framepos_t find_next_transient (framepos_t position, int dir);
 

libs/ardour/ardour/region.h

@@ -197,7 +197,8 @@ class LIBARDOUR_API Region
 	bool region_list_equivalent (boost::shared_ptr<const Region>) const;
 	bool source_equivalent (boost::shared_ptr<const Region>) const;
 	bool any_source_equivalent (boost::shared_ptr<const Region>) const;
-	bool uses_source (boost::shared_ptr<const Source>) const;
+	bool uses_source (boost::shared_ptr<const Source>, bool shallow = false) const;
+	void deep_sources (std::set<boost::shared_ptr<Source> >&) const;
 
 	std::string source_string () const;
 

libs/ardour/ardour/session_playlists.h

@@ -54,12 +54,14 @@ public:
 	uint32_t source_use_count (boost::shared_ptr<const Source> src) const;
 	uint32_t region_use_count (boost::shared_ptr<Region> region) const;
 	template<class T> void foreach (T *obj, void (T::*func)(boost::shared_ptr<Playlist>));
+	void foreach (boost::function<void(boost::shared_ptr<const Playlist>)> functor);
 	void get (std::vector<boost::shared_ptr<Playlist> >&) const;
 	void unassigned (std::list<boost::shared_ptr<Playlist> > & list);
 	void destroy_region (boost::shared_ptr<Region>);
 	boost::shared_ptr<Crossfade> find_crossfade (const PBD::ID &);
 	void sync_all_regions_with_regions ();
 	std::vector<boost::shared_ptr<Playlist> > playlists_for_track (boost::shared_ptr<Track>) const;
+	uint32_t n_playlists() const;
 
 private:
 	friend class Session;
@@ -69,7 +71,6 @@ private:
 	void remove_weak (boost::weak_ptr<Playlist>);
 	void track (bool, boost::weak_ptr<Playlist>);
 
-	uint32_t n_playlists() const;
 	void find_equivalent_playlist_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >& result);
 	void update_after_tempo_map_change ();
 	void add_state (XMLNode *, bool);

libs/ardour/playlist.cc

@@ -1774,12 +1774,23 @@ Playlist::region_bounds_changed (const PropertyChange& what_changed, boost::shar
   **********************************************************************/
 
 boost::shared_ptr<RegionList>
-Playlist::region_list() {
+Playlist::region_list()
+{
 	RegionReadLock rlock (this);
 	boost::shared_ptr<RegionList> rlist (new RegionList (regions.rlist ()));
 	return rlist;
 }
 
+void
+Playlist::deep_sources (std::set<boost::shared_ptr<Source> >& sources) const
+{
+	RegionReadLock rlock (const_cast<Playlist*>(this));
+
+	for (RegionList::const_iterator i = regions.begin(); i != regions.end(); ++i) {
+		(*i)->deep_sources (sources);
+	}
+}
+
 boost::shared_ptr<RegionList>
 Playlist::regions_at (framepos_t frame)
 {
@@ -2659,12 +2670,12 @@ Playlist::nudge_after (framepos_t start, framecnt_t distance, bool forwards)
 }
 
 bool
-Playlist::uses_source (boost::shared_ptr<const Source> src) const
+Playlist::uses_source (boost::shared_ptr<const Source> src, bool shallow) const
 {
 	RegionReadLock rlock (const_cast<Playlist*> (this));
 
 	for (set<boost::shared_ptr<Region> >::const_iterator r = all_regions.begin(); r != all_regions.end(); ++r) {
-		if ((*r)->uses_source (src)) {
+		if ((*r)->uses_source (src, true)) {
 			return true;
 		}
 	}
@@ -2672,6 +2683,7 @@ Playlist::uses_source (boost::shared_ptr<const Source> src) const
 	return false;
 }
 
+
 boost::shared_ptr<Region>
 Playlist::find_region (const ID& id) const
 {
@@ -2936,25 +2948,6 @@ Playlist::has_region_at (framepos_t const p) const
 	return (i != regions.end());
 }
 
-/** Remove any region that uses a given source */
-void
-Playlist::remove_region_by_source (boost::shared_ptr<Source> s)
-{
-	RegionWriteLock rl (this);
-
-	RegionList::iterator i = regions.begin();
-	while (i != regions.end()) {
-		RegionList::iterator j = i;
-		++j;
-
-		if ((*i)->uses_source (s)) {
-			remove_region_internal (*i);
-		}
-
-		i = j;
-	}
-}
-
 /** Look from a session frame time and find the start time of the next region
  *  which is on the top layer of this playlist.
  *  @param t Time to look from.

libs/ardour/region.cc

@@ -81,55 +81,55 @@ void
 Region::make_property_quarks ()
 {
 	Properties::muted.property_id = g_quark_from_static_string (X_("muted"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for muted = %1\n", 	Properties::muted.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for muted = %1\n",	Properties::muted.property_id));
 	Properties::opaque.property_id = g_quark_from_static_string (X_("opaque"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for opaque = %1\n", 	Properties::opaque.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for opaque = %1\n",	Properties::opaque.property_id));
 	Properties::locked.property_id = g_quark_from_static_string (X_("locked"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for locked = %1\n", 	Properties::locked.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for locked = %1\n",	Properties::locked.property_id));
 	Properties::video_locked.property_id = g_quark_from_static_string (X_("video-locked"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for video-locked = %1\n", 	Properties::video_locked.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for video-locked = %1\n",	Properties::video_locked.property_id));
 	Properties::automatic.property_id = g_quark_from_static_string (X_("automatic"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for automatic = %1\n", 	Properties::automatic.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for automatic = %1\n",	Properties::automatic.property_id));
 	Properties::whole_file.property_id = g_quark_from_static_string (X_("whole-file"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for whole-file = %1\n", 	Properties::whole_file.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for whole-file = %1\n",	Properties::whole_file.property_id));
 	Properties::import.property_id = g_quark_from_static_string (X_("import"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for import = %1\n", 	Properties::import.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for import = %1\n",	Properties::import.property_id));
 	Properties::external.property_id = g_quark_from_static_string (X_("external"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for external = %1\n", 	Properties::external.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for external = %1\n",	Properties::external.property_id));
 	Properties::sync_marked.property_id = g_quark_from_static_string (X_("sync-marked"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for sync-marked = %1\n", 	Properties::sync_marked.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for sync-marked = %1\n",		Properties::sync_marked.property_id));
 	Properties::left_of_split.property_id = g_quark_from_static_string (X_("left-of-split"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for left-of-split = %1\n", 	Properties::left_of_split.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for left-of-split = %1\n",	Properties::left_of_split.property_id));
 	Properties::right_of_split.property_id = g_quark_from_static_string (X_("right-of-split"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for right-of-split = %1\n", 	Properties::right_of_split.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for right-of-split = %1\n",	Properties::right_of_split.property_id));
 	Properties::hidden.property_id = g_quark_from_static_string (X_("hidden"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for hidden = %1\n", 	Properties::hidden.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for hidden = %1\n",	Properties::hidden.property_id));
 	Properties::position_locked.property_id = g_quark_from_static_string (X_("position-locked"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position-locked = %1\n", 	Properties::position_locked.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position-locked = %1\n",	Properties::position_locked.property_id));
 	Properties::valid_transients.property_id = g_quark_from_static_string (X_("valid-transients"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for valid-transients = %1\n", 	Properties::valid_transients.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for valid-transients = %1\n",	Properties::valid_transients.property_id));
 	Properties::start.property_id = g_quark_from_static_string (X_("start"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for start = %1\n", 	Properties::start.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for start = %1\n",	Properties::start.property_id));
 	Properties::length.property_id = g_quark_from_static_string (X_("length"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for length = %1\n", 	Properties::length.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for length = %1\n",	Properties::length.property_id));
 	Properties::position.property_id = g_quark_from_static_string (X_("position"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position = %1\n", 	Properties::position.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position = %1\n",	Properties::position.property_id));
 	Properties::sync_position.property_id = g_quark_from_static_string (X_("sync-position"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for sync-position = %1\n", 	Properties::sync_position.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for sync-position = %1\n",	Properties::sync_position.property_id));
 	Properties::layer.property_id = g_quark_from_static_string (X_("layer"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for layer = %1\n", 	Properties::layer.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for layer = %1\n",	Properties::layer.property_id));
 	Properties::ancestral_start.property_id = g_quark_from_static_string (X_("ancestral-start"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for ancestral-start = %1\n", 	Properties::ancestral_start.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for ancestral-start = %1\n",	Properties::ancestral_start.property_id));
 	Properties::ancestral_length.property_id = g_quark_from_static_string (X_("ancestral-length"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for ancestral-length = %1\n", 	Properties::ancestral_length.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for ancestral-length = %1\n",	Properties::ancestral_length.property_id));
 	Properties::stretch.property_id = g_quark_from_static_string (X_("stretch"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for stretch = %1\n", 	Properties::stretch.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for stretch = %1\n",	Properties::stretch.property_id));
 	Properties::shift.property_id = g_quark_from_static_string (X_("shift"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for shift = %1\n", 	Properties::shift.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for shift = %1\n",	Properties::shift.property_id));
 	Properties::position_lock_style.property_id = g_quark_from_static_string (X_("positional-lock-style"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position_lock_style = %1\n", 	Properties::position_lock_style.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for position_lock_style = %1\n",		Properties::position_lock_style.property_id));
 	Properties::layering_index.property_id = g_quark_from_static_string (X_("layering-index"));
-	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for layering_index = %1\n", 	Properties::layering_index.property_id));
+	DEBUG_TRACE (DEBUG::Properties, string_compose ("quark for layering_index = %1\n",	Properties::layering_index.property_id));
 }
 
 void
@@ -1522,19 +1522,59 @@ Region::source_string () const
 	return res.str();
 }
 
+void
+Region::deep_sources (std::set<boost::shared_ptr<Source> > & sources) const
+{
+	for (SourceList::const_iterator i = _sources.begin(); i != _sources.end(); ++i) {
+
+		boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
+
+		if (ps) {
+			if (sources.find (ps) == sources.end()) {
+				ps->playlist()->deep_sources (sources);
+				cerr << ps->name() << " new source\n";
+			} else {
+				cerr << ps->name() << " already in source list\n";
+			}
+		}
+
+		sources.insert (*i);
+		cerr << "Added src " << (*i)->name() << endl;
+	}
+
+	for (SourceList::const_iterator i = _master_sources.begin(); i != _master_sources.end(); ++i) {
+
+		boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
+
+		if (ps) {
+			if (sources.find (ps) == sources.end()) {
+				ps->playlist()->deep_sources (sources);
+				cerr << ps->name() << " new source2\n";
+			} else {
+				cerr << ps->name() << " already in source list2\n";
+			}
+		}
+
+		sources.insert (*i);
+		cerr << "Added master src " << (*i)->name() << endl;
+	}
+}
+
 bool
-Region::uses_source (boost::shared_ptr<const Source> source) const
+Region::uses_source (boost::shared_ptr<const Source> source, bool shallow) const
 {
 	for (SourceList::const_iterator i = _sources.begin(); i != _sources.end(); ++i) {
 		if (*i == source) {
 			return true;
 		}
 
-		boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
+		if (!shallow) {
+			boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
 
-		if (ps) {
+			if (ps) {
-			if (ps->playlist()->uses_source (source)) {
+				if (ps->playlist()->uses_source (source)) {
-				return true;
+					return true;
+				}
 			}
 		}
 	}
@@ -1544,11 +1584,13 @@ Region::uses_source (boost::shared_ptr<const Source> source) const
 			return true;
 		}
 
-		boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
+		if (!shallow) {
+			boost::shared_ptr<PlaylistSource> ps = boost::dynamic_pointer_cast<PlaylistSource> (*i);
 
-		if (ps) {
+			if (ps) {
-			if (ps->playlist()->uses_source (source)) {
+				if (ps->playlist()->uses_source (source)) {
-				return true;
+					return true;
+				}
 			}
 		}
 	}
@@ -1556,6 +1598,7 @@ Region::uses_source (boost::shared_ptr<const Source> source) const
 	return false;
 }
 
+
 framecnt_t
 Region::source_length(uint32_t n) const
 {

libs/ardour/region_factory.cc

@@ -636,7 +636,7 @@ RegionFactory::get_regions_using_source (boost::shared_ptr<Source> s, std::set<b
 {
 	Glib::Threads::Mutex::Lock lm (region_map_lock);
 
-	for (RegionMap::iterator i = region_map.begin(); i != region_map.end(); ++i) {
+	for (RegionMap::const_iterator i = region_map.begin(); i != region_map.end(); ++i) {
 		if (i->second->uses_source (s)) {
 			r.insert (i->second);
 		}

libs/ardour/session_playlists.cc

@@ -260,6 +260,7 @@ SessionPlaylists::source_use_count (boost::shared_ptr<const Source> src) const
 {
 	uint32_t count = 0;
 
+	cerr << "\t\tcheck " << playlists.size() << " playlists\n";
 	for (List::const_iterator p = playlists.begin(); p != playlists.end(); ++p) {
                 if ((*p)->uses_source (src)) {
                         ++count;
@@ -267,6 +268,7 @@ SessionPlaylists::source_use_count (boost::shared_ptr<const Source> src) const
                 }
 	}
 
+	cerr << "\t\tcheck " << playlists.size() << " unused playlists\n";
 	for (List::const_iterator p = unused_playlists.begin(); p != unused_playlists.end(); ++p) {
                 if ((*p)->uses_source (src)) {
                         ++count;
@@ -500,3 +502,20 @@ SessionPlaylists::playlists_for_track (boost::shared_ptr<Track> tr) const
 
 	return pl_tr;
 }
+
+void
+SessionPlaylists::foreach (boost::function<void(boost::shared_ptr<const Playlist>)> functor)
+{
+	Glib::Threads::Mutex::Lock lm (lock);
+	for (List::iterator i = playlists.begin(); i != playlists.end(); i++) {
+		if (!(*i)->hidden()) {
+			functor (*i);
+		}
+	}
+	for (List::iterator i = unused_playlists.begin(); i != unused_playlists.end(); i++) {
+		if (!(*i)->hidden()) {
+			functor (*i);
+		}
+	}
+}
+

libs/ardour/session_state.cc

@@ -2872,14 +2872,17 @@ Session::find_all_sources_across_snapshots (set<string>& result, bool exclude_th
 		return 0;
 	}
 
-	this_snapshot_path = _path;
+	this_snapshot_path = Glib::build_filename (_path, legalize_for_path (_current_snapshot_name));
-	this_snapshot_path += legalize_for_path (_current_snapshot_name);
 	this_snapshot_path += statefile_suffix;
 
 	for (vector<string>::iterator i = state_files.begin(); i != state_files.end(); ++i) {
 
+		cerr << "Looking at snapshot " << (*i) << " ( with this = [" << this_snapshot_path << "])\n";
+
 		if (exclude_this_snapshot && *i == this_snapshot_path) {
+			cerr << "\texcluded\n";
 			continue;
+
 		}
 
 		if (find_all_sources (*i, result) < 0) {
@@ -3010,6 +3013,12 @@ Session::cleanup_peakfiles ()
 	return 0;
 }
 
+static void
+merge_all_sources (boost::shared_ptr<const Playlist> pl, std::set<boost::shared_ptr<Source> >* all_sources)
+{
+	pl->deep_sources (*all_sources);
+}
+
 int
 Session::cleanup_sources (CleanupReport& rep)
 {
@@ -3020,14 +3029,14 @@ Session::cleanup_sources (CleanupReport& rep)
 	string midi_path;
 	vector<string> candidates;
 	vector<string> unused;
-	set<string> all_sources;
+	set<string> sources_used_by_all_snapshots;
-	bool used;
 	string spath;
 	int ret = -1;
 	string tmppath1;
 	string tmppath2;
 	Searchpath asp;
 	Searchpath msp;
+	set<boost::shared_ptr<Source> > sources_used_by_this_snapshot;
 
 	_state_of_the_state = (StateOfTheState) (_state_of_the_state | InCleanup);
 
@@ -3098,12 +3107,21 @@ Session::cleanup_sources (CleanupReport& rep)
 	find_files_matching_filter (candidates, audio_path, accept_all_audio_files, (void *) 0, true, true);
 	find_files_matching_filter (candidates, midi_path, accept_all_midi_files, (void *) 0, true, true);
 
-	/* find all sources, but don't use this snapshot because the
+	/* add sources from all other snapshots as "used", but don't use this
-	   state file on disk still references sources we may have already
+	   snapshot because the state file on disk still references sources we
-	   dropped.
+	   may have already dropped.
 	*/
 
-	find_all_sources_across_snapshots (all_sources, true);
+	find_all_sources_across_snapshots (sources_used_by_all_snapshots, true);
+
+	/* Although the region factory has a list of all regions ever created
+	 * for this session, we're only interested in regions actually in
+	 * playlists right now. So merge all playlist regions lists together.
+	 *
+	 * This will include the playlists used within compound regions.
+	 */
+
+	playlists->foreach (boost::bind (merge_all_sources, _1, &sources_used_by_this_snapshot));
 
 	/*  add our current source list
 	 */
@@ -3113,59 +3131,76 @@ Session::cleanup_sources (CleanupReport& rep)
                 SourceMap::iterator tmp = i;
                 ++tmp;
 
-		if ((fs = boost::dynamic_pointer_cast<FileSource> (i->second)) != 0) {
+		if ((fs = boost::dynamic_pointer_cast<FileSource> (i->second)) == 0) {
+			/* not a file */
+			i = tmp;
+			continue;
+		}
 
-			/* this is mostly for windows which doesn't allow file
+		/* this is mostly for windows which doesn't allow file
-			 * renaming if the file is in use. But we don't special
+		 * renaming if the file is in use. But we do not special
-			 * case it because we need to know if this causes
+		 * case it because we need to know if this causes
-			 * problems, and the easiest way to notice that is to
+		 * problems, and the easiest way to notice that is to
-			 * keep it in place for all platforms.
+		 * keep it in place for all platforms.
+		 */
+
+		fs->close ();
+
+		if (!fs->is_stub()) {
+
+			/* Note that we're checking a list of all
+			 * sources across all snapshots with the list
+			 * of sources used by this snapshot.
 			 */
 
-			fs->close ();
+			if (sources_used_by_this_snapshot.find (i->second) != sources_used_by_this_snapshot.end()) {
+				/* this source is in use by this snapshot */
+				sources_used_by_all_snapshots.insert (fs->path());
+				cerr << "Source from source list found in used_by_this_snapshot (" << fs->path() << ")\n";
+			} else {
+				cerr << "Source from source list NOT found in used_by_this_snapshot (" << fs->path() << ")\n";
+				/* this source is NOT in use by this snapshot
+				 */
 
-			if (!fs->is_stub()) {
+				/* remove all related regions from RegionFactory master list
+				 */
 
-				if (playlists->source_use_count (fs) != 0) {
+				RegionFactory::remove_regions_using_source (i->second);
-					all_sources.insert (fs->path());
-				} else {
 
-					/* we might not remove this source from disk, because it may be used
+				/* remove from our current source list
-					   by other snapshots, but its not being used in this version
+				 * also. We may not remove it from
-					   so lets get rid of it now, along with any representative regions
+				 * disk, because it may be used by
-					   in the region list.
+				 * other snapshots, but it isn't used inside this
-					*/
+				 * snapshot anymore, so we don't need a
+				 * reference to it.
+				 */
 
-					RegionFactory::remove_regions_using_source (i->second);
+				sources.erase (i);
-
-					// also remove source from all_sources
-
-					for (set<string>::iterator j = all_sources.begin(); j != all_sources.end(); ++j) {
-						spath = Glib::path_get_basename (*j);
-						if (spath == i->second->name()) {
-							all_sources.erase (j);
-							break;
-						}
-					}
-
-					sources.erase (i);
-				}
 			}
 		}
 
                 i = tmp;
 	}
 
+	/* now check each candidate source to see if it exists in the list of
+	   sources_used_by_all_snapshots. If it doesn't, put it into "unused".
+	*/
+
+	cerr << "Candidates: " << candidates.size() << endl;
+	cerr << "Used by others: " << sources_used_by_all_snapshots.size() << endl;
+
 	for (vector<string>::iterator x = candidates.begin(); x != candidates.end(); ++x) {
 
-		used = false;
+		bool used = false;
 		spath = *x;
 
-		for (set<string>::iterator i = all_sources.begin(); i != all_sources.end(); ++i) {
+		for (set<string>::iterator i = sources_used_by_all_snapshots.begin(); i != sources_used_by_all_snapshots.end(); ++i) {
 
 			tmppath1 = canonical_path (spath);
 			tmppath2 = canonical_path ((*i));
 
+			cerr << "\t => " << tmppath2 << endl;
+
 			if (tmppath1 == tmppath2) {
 				used = true;
 				break;
@@ -3177,6 +3212,14 @@ Session::cleanup_sources (CleanupReport& rep)
 		}
 	}
 
+	cerr << "Actually unused: " << unused.size() << endl;
+
+	if (unused.empty()) {
+		/* Nothing to do */
+		ret = 0;
+		goto out;
+	}
+
 	/* now try to move all unused files into the "dead" directory(ies) */
 
 	for (vector<string>::iterator x = unused.begin(); x != unused.end(); ++x) {
@@ -3239,19 +3282,13 @@ Session::cleanup_sources (CleanupReport& rep)
 				newpath = newpath_v;
 			}
 
-		} else {
-
-			/* it doesn't exist, or we can't read it or something */
-
 		}
 
 		g_stat ((*x).c_str(), &statbuf);
 
 		if (::rename ((*x).c_str(), newpath.c_str()) != 0) {
-			error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"),
+			error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"), (*x), newpath, strerror (errno)) << endmsg;
-					  (*x), newpath, strerror (errno))
+			continue;
-			      << endmsg;
-			goto out;
 		}
 
 		/* see if there an easy to find peakfile for this file, and remove it.