From 7d8c9cc7e31e15b93af7c08f7d03ed8ca10f5454 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 7 Oct 2008 22:11:16 +0200
Subject: directory: serialize song deletes from playlist during update

This makes the update code thread-safe and doesn't penalize
the playlist code by complicating it with complicated and
error-prone locks (and the associated overhead, not everybody
has a thread-implementation as good as NPTL).

The update task blocks during the delete; but the update task is
a slow task anyways so we can block w/o people caring too much.

This was also our only freeSong call site, so remove that
function.

Note that deleting entire directories is not fully thread-safe,
yet; as their traversals are not yet locked.
---
 src/directory.c | 31 ++++++++++++++++++++++++++++---
 src/song.c      |  6 ------
 src/song.h      |  2 --
 3 files changed, 28 insertions(+), 11 deletions(-)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index fb8c23e2f..949d249a4 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -31,6 +31,7 @@
 #include "song_save.h"
 #include "main_notify.h"
 #include "dirvec.h"
+#include "condition.h"
 
 #define DIRECTORY_DIR		"directory: "
 #define DIRECTORY_MTIME		"mtime: " /* DEPRECATED, noop-read-only */
@@ -64,8 +65,13 @@ static time_t directory_dbModTime;
 static pthread_t update_thr;
 
 static const int update_task_id_max = 1 << 15;
+
 static int update_task_id;
 
+static Song *delete;
+
+static struct condition delete_cond;
+
 static enum update_return
 addToDirectory(Directory * directory, const char *name);
 
@@ -146,6 +152,16 @@ void reap_update_task(void)
 
 	assert(pthread_equal(pthread_self(), main_task));
 
+	cond_enter(&delete_cond);
+	if (delete) {
+		char tmp[MPD_PATH_MAX];
+		LOG("removing: %s\n", get_song_url(tmp, delete));
+		deleteASongFromPlaylist(delete);
+		delete = NULL;
+		cond_signal_async(&delete_cond);
+	}
+	cond_leave(&delete_cond);
+
 	if (progress != UPDATE_PROGRESS_DONE)
 		return;
 	if (pthread_join(update_thr, (void **)&ret))
@@ -217,10 +233,19 @@ static void freeDirectory(Directory * directory)
 
 static void delete_song(Directory *dir, Song *del)
 {
-	char path_max_tmp[MPD_PATH_MAX]; /* wasteful */
-	LOG("removing: %s\n", get_song_url(path_max_tmp, del));
+	/* first, prevent traversers in main task from getting this */
 	songvec_delete(&dir->songs, del);
-	freeSong(del); /* FIXME racy */
+
+	/* now take it out of the playlist (in the main_task) */
+	cond_enter(&delete_cond);
+	assert(!delete);
+	delete = del;
+	wakeup_main_task();
+	do { cond_wait(&delete_cond); } while (delete);
+	cond_leave(&delete_cond);
+
+	/* finally, all possible references gone, free it */
+	freeJustSong(del);
 }
 
 static void deleteEmptyDirectoriesInDirectory(Directory * directory)
diff --git a/src/song.c b/src/song.c
index acc19eb0e..f125cf2df 100644
--- a/src/song.c
+++ b/src/song.c
@@ -79,12 +79,6 @@ Song *newSong(const char *url, Directory * parentDir)
 	return song;
 }
 
-void freeSong(Song * song)
-{
-	deleteASongFromPlaylist(song);
-	freeJustSong(song);
-}
-
 void freeJustSong(Song * song)
 {
 	if (song->tag)
diff --git a/src/song.h b/src/song.h
index b5f316955..fddd5ccfd 100644
--- a/src/song.h
+++ b/src/song.h
@@ -42,8 +42,6 @@ song_alloc(const char *url, struct _Directory *parent);
 
 Song *newSong(const char *url, struct _Directory *parentDir);
 
-void freeSong(Song *);
-
 void freeJustSong(Song *);
 
 int updateSongInfo(Song * song);
-- 
cgit v1.2.3