From ada24f9a921ff95d874195acf253b5a9dd12213d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 22 Sep 2008 02:04:23 -0700 Subject: 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 | 18 ++---- src/directory.c | 158 +++++++++++++++++------------------------------------ src/directory.h | 2 +- src/main.c | 2 +- src/sig_handlers.c | 1 - 5 files changed, 56 insertions(+), 125 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index ebca41bde..f5996c576 100644 --- a/src/command.c +++ b/src/command.c @@ -805,13 +805,10 @@ static int listHandleUpdate(int fd, 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 @@ -820,12 +817,8 @@ static int listHandleUpdate(int fd, if (next) nextCmd = getCommandEntryFromString(next->data, permission); - if (cmd != nextCmd) { - int ret = updateInit(fd, pathList); - freeList(pathList); - pathList = NULL; - return ret; - } + if (cmd != nextCmd) + return updateInit(fd, pathList); return 0; } @@ -834,12 +827,9 @@ static int handleUpdate(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { if (argc == 2) { - int ret; List *pathList = makeList(NULL, 1); insertInList(pathList, argv[1], NULL); - ret = updateInit(fd, pathList); - freeList(pathList); - return ret; + return updateInit(fd, pathList); } return updateInit(fd, NULL); } diff --git a/src/directory.c b/src/directory.c index 08b3fb0e2..cb1995a93 100644 --- a/src/directory.c +++ b/src/directory.c @@ -54,17 +54,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 sig_atomic_t update_exited; - -static sig_atomic_t update_status; +static pthread_t update_thr; -static sig_atomic_t directory_updateJobId; +static int directory_updateJobId; static DirectoryList *newDirectoryList(void); @@ -116,127 +118,67 @@ 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) { - update_status = status; - update_exited = 1; - wakeup_main_task(); - } + if (progress != UPDATE_PROGRESS_DONE) + return; + pthread_join(update_thr, NULL); + progress = UPDATE_PROGRESS_IDLE; } -void readDirectoryDBIfUpdateIsFinished(void) +static void * update_task(void *arg) { - int status; - - if (!update_exited) - return; + List *path_list = (List *)arg; + enum update_return ret = UPDATE_RETURN_NOUPDATE; - 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"); + if (path_list) { + ListNode *node = path_list->firstNode; + + while (node) { + switch (updatePath(node->key)) { + case UPDATE_RETURN_ERROR: + ret = UPDATE_RETURN_ERROR; + goto out; + case UPDATE_RETURN_NOUPDATE: + break; + case UPDATE_RETURN_UPDATED: + ret = UPDATE_RETURN_UPDATED; + } + node = node->nextNode; } + free(path_list); + } else { + ret = updateDirectory(mp3rootDirectory); } - update_exited = 0; - directory_updatePid = 0; + + if (ret == UPDATE_RETURN_UPDATED && writeDirectoryDB() < 0) + ret = UPDATE_RETURN_ERROR; +out: + progress = UPDATE_PROGRESS_DONE; + wakeup_main_task(); + return (void *)ret; } -int updateInit(int fd, List * pathList) +int updateInit(int fd, List * path_list) { - if (directory_updatePid > 0) { - commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); - return -1; - } - - /* - * 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) { - /* child */ - enum update_return dbUpdated = UPDATE_RETURN_NOUPDATE; - - unblockSignals(); - - finishSigHandlers(); - closeAllListenSockets(); - client_manager_deinit(); - 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; - - while (node) { - switch (updatePath(node->key)) { - case 1: - dbUpdated = UPDATE_RETURN_UPDATED; - break; - case 0: - break; - default: - exit(DIRECTORY_UPDATE_EXIT_ERROR); - } - node = node->nextNode; - } - } else { - if ((dbUpdated = updateDirectory(mp3rootDirectory)) < 0) - exit(DIRECTORY_UPDATE_EXIT_ERROR); - } + pthread_attr_t attr; - if (dbUpdated == UPDATE_RETURN_NOUPDATE) - exit(DIRECTORY_UPDATE_EXIT_NOUPDATE); - - /* 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"); - commandError(fd, ACK_ERROR_SYSTEM, - "problems trying to update"); - directory_updatePid = 0; + if (progress != UPDATE_PROGRESS_IDLE) { + commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); return -1; } - unblockSignals(); + 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); fdprintf(fd, "updating_db: %i\n", (int)directory_updateJobId); diff --git a/src/directory.h b/src/directory.h index 40c9d6499..351ebd2cd 100644 --- a/src/directory.h +++ b/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); diff --git a/src/main.c b/src/main.c index dcfb7f64d..8028505ab 100644 --- a/src/main.c +++ b/src/main.c @@ -439,7 +439,7 @@ int main(int argc, char *argv[]) COMMAND_RETURN_KILL != handlePendingSignals()) { syncPlayerAndPlaylist(); client_manager_expire(); - readDirectoryDBIfUpdateIsFinished(); + reap_update_task(); } write_state_file(); diff --git a/src/sig_handlers.c b/src/sig_handlers.c index 6b28cb675..e4ac21f22 100644 --- a/src/sig_handlers.c +++ b/src/sig_handlers.c @@ -57,7 +57,6 @@ static void chldSigHandler(mpd_unused int sig) else break; } - directory_sigChldHandler(pid, status); } } -- cgit v1.2.3