directory: serialize freeSong() within the main thread

It's possible the playlist will be accessing a song that is to
be freed in the update thread.  Rather than going through the
complexity (and potential to make mistakes) of locking the
playlist (as well as losing CPU cycles/pipelining due to
barriers with mutexes), we'll just line up all songs to
be freed in the main thread.

It's relatively uncommon to call freeSong() heavily (as it is to
update); so the extra, temporary memory usage won't be very
noticeable.

Additionally, if a song is renamed and it contains unique tag
item; this has the additional side effect of preventing
unnecessary fragmentation where an item is freed and shortly
reallocated.

1 files changed, 10 insertions, 1 deletions

diff --git a/src/directory.c b/src/directory.c
index fa0856dc5..e7210816f 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -68,6 +68,8 @@ static pthread_t update_thr;
 
 static int directory_updateJobId;
 
+static struct songvec reap_songs;
+
 static DirectoryList *newDirectoryList(void);
 
 static int addToDirectory(Directory * directory,
@@ -125,6 +127,13 @@ void reap_update_task(void)
 {
 	if (progress != UPDATE_PROGRESS_DONE)
 		return;
+	if (reap_songs.base) {
+		int i;
+		for (i = reap_songs.nr; --i >= 0; )
+			freeSong(reap_songs.base[i]);
+		reap_songs.nr = 0;
+		reap_songs.base = NULL;
+	}
 	pthread_join(update_thr, NULL);
 	progress = UPDATE_PROGRESS_IDLE;
 }
@@ -241,7 +250,7 @@ static void removeSongFromDirectory(Directory * directory, const char *shortname
 		char path_max_tmp[MPD_PATH_MAX]; /* wasteful */
 		LOG("removing: %s\n", get_song_url(path_max_tmp, song));
 		songvec_delete(&directory->songs, song);
-		freeSong(song);
+		songvec_add(&reap_songs, song);
 	}
 }