From 66a934d068fb6e0aab2043d56a8857d57b6b7596 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 20 Sep 2008 17:22:15 -0700
Subject: workaround race condition on updates with broken signal blocking

pthreads with our existing signal blocking/handling is broken,
for now just sleep a bit in the child to prevent the CHLD handler
from being called too early.  Also, improve error reporting when
handling SIGCHLD by storing the status to be called in the main
task (which can be logged, since we can't do logging inside the
sig handler).
---
 src/directory.c | 89 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/src/directory.c b/src/directory.c
index c9361fb18..230dea5dc 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -33,6 +33,7 @@
 #include "ack.h"
 #include "myfprintf.h"
 #include "dbUtils.h"
+#include "main_notify.h"
 
 #define DIRECTORY_DIR		"directory: "
 #define DIRECTORY_MTIME		"mtime: "
@@ -55,11 +56,13 @@ static Directory *mp3rootDirectory;
 
 static time_t directory_dbModTime;
 
-static volatile int directory_updatePid;
+static sig_atomic_t directory_updatePid;
 
-static volatile int directory_reReadDB;
+static sig_atomic_t update_exited;
 
-static volatile mpd_uint16 directory_updateJobId;
+static sig_atomic_t update_status;
+
+static sig_atomic_t directory_updateJobId;
 
 static DirectoryList *newDirectoryList(void);
 
@@ -109,53 +112,48 @@ static char *getDbFile(void)
 	return param->value;
 }
 
-static void clearUpdatePid(void)
-{
-	directory_updatePid = 0;
-}
-
 int isUpdatingDB(void)
 {
-	if (directory_updatePid > 0 || directory_reReadDB) {
-		return directory_updateJobId;
-	}
-	return 0;
+	return directory_updatePid > 0 ? directory_updateJobId : 0;
 }
 
 void directory_sigChldHandler(int pid, int status)
 {
 	if (directory_updatePid == pid) {
-		if (WIFSIGNALED(status) && WTERMSIG(status) != SIGTERM) {
-			/* ERROR("update process died from a "
-			      "non-TERM signal: %i\n", WTERMSIG(status)); */
-		} else if (!WIFSIGNALED(status)) {
-			switch (WEXITSTATUS(status)) {
-			case DIRECTORY_UPDATE_EXIT_UPDATE:
-				directory_reReadDB = 1;
-				/* DEBUG("directory_sigChldHandler: "
-				      "updated db\n"); */
-			case DIRECTORY_UPDATE_EXIT_NOUPDATE:
-				/* DEBUG("directory_sigChldHandler: "
-				      "update exited succesffully\n"); */
-				break;
-			/*
-			default:
-				ERROR("error updating db\n");
-			*/
-			}
-		}
-		clearUpdatePid();
+		update_status = status;
+		update_exited = 1;
+		wakeup_main_task();
 	}
 }
 
 void readDirectoryDBIfUpdateIsFinished(void)
 {
-	if (directory_reReadDB && 0 == directory_updatePid) {
-		DEBUG("readDirectoryDB since update finished successfully\n");
-		readDirectoryDB();
-		playlistVersionChange();
-		directory_reReadDB = 0;
+	int status;
+
+	if (!update_exited)
+		return;
+
+	status = update_status;
+
+	if (WIFSIGNALED(status) && WTERMSIG(status) != SIGTERM) {
+		ERROR("update process died from a non-TERM signal: %d\n",
+		      WTERMSIG(status));
+	} else if (!WIFSIGNALED(status)) {
+		switch (WEXITSTATUS(status)) {
+		case DIRECTORY_UPDATE_EXIT_UPDATE:
+			DEBUG("update finished successfully with changes\n");
+			readDirectoryDB();
+			DEBUG("update changes read into memory\n");
+			playlistVersionChange();
+		case DIRECTORY_UPDATE_EXIT_NOUPDATE:
+			DEBUG("update exited successfully with no changes\n");
+			break;
+		default:
+			ERROR("error updating db\n");
+		}
 	}
+	update_exited = 0;
+	directory_updatePid = 0;
 }
 
 int updateInit(int fd, List * pathList)
@@ -165,8 +163,14 @@ int updateInit(int fd, List * pathList)
 		return -1;
 	}
 
-	/* need to block CHLD signal, cause it can exit before we
-	   even get a chance to assign directory_updatePID */
+	/*
+	 * need to block CHLD signal, cause it can exit before we
+	 * even get a chance to assign directory_updatePID
+	 *
+	 * Update: our signal blocking is is utterly broken by
+	 * pthreads(); goal will be to remove dependency on signals;
+	 * but for now use the my_usleep hack below.
+	 */
 	blockSignals();
 	directory_updatePid = fork();
 	if (directory_updatePid == 0) {
@@ -181,6 +185,13 @@ int updateInit(int fd, List * pathList)
 		finishPlaylist();
 		finishVolume();
 
+		/*
+		 * XXX HACK to workaround race condition where
+		 * directory_updatePid is still zero in the parent even upon
+		 * entry of directory_sigChldHandler.
+		 */
+		my_usleep(100000);
+
 		if (pathList) {
 			ListNode *node = pathList->firstNode;
 
-- 
cgit v1.2.3