directory: update do its work inside a thread

A lot of the preparation was needed (and done in previous
months) in making update thread-safe, but here it is.

This was the first thing I made work inside a thread when I
started mpd-uclinux many years ago, and also the last thing I've
done in mainline mpd to work inside a thread, go figure.

src/command.c

@@ -819,13 +819,10 @@ static int listHandleUpdate(struct client *client,
 			    char *argv[],
 			    struct strnode *cmdnode, CommandEntry * cmd)
 {
-	static List *pathList;
+	List *pathList = makeList(NULL, 1);
 	CommandEntry *nextCmd = NULL;
 	struct strnode *next = cmdnode->next;
 
-	if (!pathList)
-		pathList = makeList(NULL, 1);
-
 	if (argc == 2)
 		insertInList(pathList, argv[1], NULL);
 	else
@@ -837,25 +834,9 @@ static int listHandleUpdate(struct client *client,
 
 	if (cmd != nextCmd) {
 		int ret = updateInit(pathList);
-		freeList(pathList);
+		if (ret == -1)
-		pathList = NULL;
-
-		switch (ret) {
-		case 0:
 			command_error(client, ACK_ERROR_UPDATE_ALREADY,
 				      "already updating");
-			break;
-
-		case -1:
-			command_error(client, ACK_ERROR_SYSTEM,
-				      "problems trying to update");
-			break;
-
-		default:
-			client_printf(client, "updating_db: %i\n", ret);
-			ret = 0;
-			break;
-		}
 
 		return ret;
 	}
@@ -872,26 +853,10 @@ static int handleUpdate(struct client *client,
 		List *pathList = makeList(NULL, 1);
 		insertInList(pathList, argv[1], NULL);
 		ret = updateInit(pathList);
-		freeList(pathList);
+		if (ret == -1)
-	} else
+			command_error(client, ACK_ERROR_UPDATE_ALREADY,
-		ret = updateInit(NULL);
+				      "already updating");
-
+		return ret;
-	switch (ret) {
-	case 0:
-		command_error(client, ACK_ERROR_UPDATE_ALREADY,
-			      "already updating");
-		ret = -1;
-		break;
-
-	case -1:
-		command_error(client, ACK_ERROR_SYSTEM,
-			      "problems trying to update");
-		break;
-
-	default:
-		client_printf(client, "updating_db: %i\n", ret);
-		ret = 0;
-		break;
 	}
 
 	return ret;

src/directory.c

@@ -53,17 +53,19 @@ enum update_return {
 	UPDATE_RETURN_UPDATED = 1
 };
 
+enum update_progress {
+	UPDATE_PROGRESS_IDLE = 0,
+	UPDATE_PROGRESS_RUNNING = 1,
+	UPDATE_PROGRESS_DONE = 2
+} progress;
+
 static Directory *mp3rootDirectory;
 
 static time_t directory_dbModTime;
 
-static sig_atomic_t directory_updatePid;
+static pthread_t update_thr;
 
-static sig_atomic_t update_exited;
+static int directory_updateJobId;
-
-static sig_atomic_t update_status;
-
-static sig_atomic_t directory_updateJobId;
 
 static DirectoryList *newDirectoryList(void);
 
@@ -116,124 +118,66 @@ static char *getDbFile(void)
 
 int isUpdatingDB(void)
 {
-	return directory_updatePid > 0 ? directory_updateJobId : 0;
+	return (progress != UPDATE_PROGRESS_IDLE) ? directory_updateJobId : 0;
 }
 
-void directory_sigChldHandler(int pid, int status)
+void reap_update_task(void)
 {
-	if (directory_updatePid == pid) {
+	if (progress != UPDATE_PROGRESS_DONE)
-		update_status = status;
-		update_exited = 1;
-		wakeup_main_task();
-	}
-}
-
-void readDirectoryDBIfUpdateIsFinished(void)
-{
-	int status;
-
-	if (!update_exited)
 		return;
-
+	pthread_join(update_thr, NULL);
-	status = update_status;
+	progress = UPDATE_PROGRESS_IDLE;
-
-	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(List * pathList)
+static void * update_task(void *arg)
 {
-	if (directory_updatePid > 0)
+	List *path_list = (List *)arg;
-		return 0;
+	enum update_return ret = UPDATE_RETURN_NOUPDATE;
 
-	/*
+	if (path_list) {
-	 * need to block CHLD signal, cause it can exit before we
+		ListNode *node = path_list->firstNode;
-	 * 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) {
-		/* child */
-		enum update_return dbUpdated = UPDATE_RETURN_NOUPDATE;
 
-		unblockSignals();
+		while (node) {
-
+			switch (updatePath(node->key)) {
-		finishSigHandlers();
+			case UPDATE_RETURN_ERROR:
-		closeAllListenSockets();
+				ret = UPDATE_RETURN_ERROR;
-		client_manager_deinit();
+				goto out;
-		finishPlaylist();
+			case UPDATE_RETURN_NOUPDATE:
-		finishVolume();
+				break;
-
+			case UPDATE_RETURN_UPDATED:
-		/*
+				ret = UPDATE_RETURN_UPDATED;
-		 * 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;
-
-			while (node) {
-				switch (updatePath(node->key)) {
-				case UPDATE_RETURN_UPDATED:
-					dbUpdated = UPDATE_RETURN_UPDATED;
-					break;
-				case UPDATE_RETURN_NOUPDATE:
-					break;
-				case UPDATE_RETURN_ERROR:
-					exit(DIRECTORY_UPDATE_EXIT_ERROR);
-				}
-				node = node->nextNode;
 			}
-		} else {
+			node = node->nextNode;
-			dbUpdated = updateDirectory(mp3rootDirectory);
-			if (dbUpdated == UPDATE_RETURN_ERROR)
-				exit(DIRECTORY_UPDATE_EXIT_ERROR);
 		}
-
+		free(path_list);
-		if (dbUpdated == UPDATE_RETURN_NOUPDATE)
+	} else {
-			exit(DIRECTORY_UPDATE_EXIT_NOUPDATE);
+		ret = updateDirectory(mp3rootDirectory);
-
-		/* ignore signals since we don't want them to corrupt the db */
-		ignoreSignals();
-		if (writeDirectoryDB() < 0) {
-			exit(DIRECTORY_UPDATE_EXIT_ERROR);
-		}
-		exit(DIRECTORY_UPDATE_EXIT_UPDATE);
-	} else if (directory_updatePid < 0) {
-		unblockSignals();
-		ERROR("updateInit: Problems forking()'ing\n");
-		directory_updatePid = 0;
-		return -1;
 	}
-	unblockSignals();
 
+	if (ret == UPDATE_RETURN_UPDATED && writeDirectoryDB() < 0)
+		ret = UPDATE_RETURN_ERROR;
+out:
+	progress = UPDATE_PROGRESS_DONE;
+	wakeup_main_task();
+	return (void *)ret;
+}
+
+int updateInit(List * path_list)
+{
+	pthread_attr_t attr;
+
+	if (progress != UPDATE_PROGRESS_IDLE)
+		return -1;
+
+	progress = UPDATE_PROGRESS_RUNNING;
+
+	pthread_attr_init(&attr);
+	if (pthread_create(&update_thr, &attr, update_task, path_list))
+		FATAL("Failed to spawn update task: %s\n", strerror(errno));
 	directory_updateJobId++;
 	if (directory_updateJobId > 1 << 15)
 		directory_updateJobId = 1;
-	DEBUG("updateInit: fork()'d update child for update job id %i\n",
+	DEBUG("updateInit: spawned update thread for update job id %i\n",
 	      (int)directory_updateJobId);
 
 	return (int)directory_updateJobId;

src/directory.h

@@ -34,7 +34,7 @@ typedef struct _Directory {
 	unsigned stat; /* not needed if ino_t == dev_t == 0 is impossible */
 } Directory;
 
-void readDirectoryDBIfUpdateIsFinished(void);
+void reap_update_task(void);
 
 int isUpdatingDB(void);
 

src/main.c

@@ -444,7 +444,7 @@ int main(int argc, char *argv[])
 	       COMMAND_RETURN_KILL != handlePendingSignals()) {
 		syncPlayerAndPlaylist();
 		client_manager_expire();
-		readDirectoryDBIfUpdateIsFinished();
+		reap_update_task();
 	}
 
 	write_state_file();

src/sig_handlers.c

@@ -57,7 +57,6 @@ static void chldSigHandler(mpd_unused int sig)
 			else
 				break;
 		}
-		directory_sigChldHandler(pid, status);
 	}
 }