From b58a82295529e775fbb900ecfb2d9104b2dafdc1 Mon Sep 17 00:00:00 2001 From: Rom Lemarchand Date: Wed, 9 Jan 2013 21:31:25 -0800 Subject: logwrapper: prevent logwrap from hanging when child dies Sometimes the read on the PTY can wait indefinitely if the child dies. By using a poll statement that monitors both the output of the child and its state we prevent this from happening. Change-Id: I51d5556c66f039bca673145ca72db262977e1689 --- logwrapper/logwrap.c | 152 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 108 insertions(+), 44 deletions(-) (limited to 'logwrapper/logwrap.c') diff --git a/logwrapper/logwrap.c b/logwrapper/logwrap.c index 302a739dd..c2b36be29 100644 --- a/logwrapper/logwrap.c +++ b/logwrapper/logwrap.c @@ -16,6 +16,9 @@ #include #include +#include +#include +#include #include #include #include @@ -28,15 +31,28 @@ #include "private/android_filesystem_config.h" #include "cutils/log.h" +#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x))) + static int fatal(const char *msg) { fprintf(stderr, "%s", msg); ALOG(LOG_ERROR, "logwrapper", "%s", msg); return -1; } -void parent(const char *tag, int parent_read, int *chld_sts) { +static int parent(const char *tag, int parent_read, int signal_fd, pid_t pid, + int *chld_sts) { int status; char buffer[4096]; + struct pollfd poll_fds[] = { + [0] = { + .fd = parent_read, + .events = POLLIN, + }, + [1] = { + .fd = signal_fd, + .events = POLLIN, + }, + }; int a = 0; // start index of unprocessed data int b = 0; // end index of unprocessed data @@ -45,60 +61,82 @@ void parent(const char *tag, int parent_read, int *chld_sts) { char *btag = basename(tag); if (!btag) btag = (char*) tag; - while ((sz = read(parent_read, &buffer[b], sizeof(buffer) - 1 - b)) > 0) { + while (1) { + if (poll(poll_fds, ARRAY_SIZE(poll_fds), -1) <= 0) { + return fatal("poll failed\n"); + } - sz += b; - // Log one line at a time - for (b = 0; b < sz; b++) { - if (buffer[b] == '\r') { - buffer[b] = '\0'; - } else if (buffer[b] == '\n') { + if (poll_fds[0].revents & POLLIN) { + sz = read(parent_read, &buffer[b], sizeof(buffer) - 1 - b); + + sz += b; + // Log one line at a time + for (b = 0; b < sz; b++) { + if (buffer[b] == '\r') { + buffer[b] = '\0'; + } else if (buffer[b] == '\n') { + buffer[b] = '\0'; + ALOG(LOG_INFO, btag, "%s", &buffer[a]); + a = b + 1; + } + } + + if (a == 0 && b == sizeof(buffer) - 1) { + // buffer is full, flush buffer[b] = '\0'; ALOG(LOG_INFO, btag, "%s", &buffer[a]); - a = b + 1; + b = 0; + } else if (a != b) { + // Keep left-overs + b -= a; + memmove(buffer, &buffer[a], b); + a = 0; + } else { + a = 0; + b = 0; } } - if (a == 0 && b == sizeof(buffer) - 1) { - // buffer is full, flush - buffer[b] = '\0'; - ALOG(LOG_INFO, btag, "%s", &buffer[a]); - b = 0; - } else if (a != b) { - // Keep left-overs - b -= a; - memmove(buffer, &buffer[a], b); - a = 0; - } else { - a = 0; - b = 0; - } + if (poll_fds[1].revents & POLLIN) { + struct signalfd_siginfo sfd_info; + pid_t wpid; + // Clear all pending signals before reading the child's status + while (read(signal_fd, &sfd_info, sizeof(sfd_info)) > 0) { + if ((pid_t)sfd_info.ssi_pid != pid) + ALOG(LOG_WARN, "logwrapper", "cleared SIGCHLD for pid %u\n", + sfd_info.ssi_pid); + } + wpid = waitpid(pid, &status, WNOHANG); + if (wpid > 0) + break; + } } + // Flush remaining data if (a != b) { buffer[b] = '\0'; ALOG(LOG_INFO, btag, "%s", &buffer[a]); } - if (wait(&status) != -1) { // Wait for child - if (WIFEXITED(status) && WEXITSTATUS(status)) + if (WIFEXITED(status)) { + if (WEXITSTATUS(status)) ALOG(LOG_INFO, "logwrapper", "%s terminated by exit(%d)", tag, WEXITSTATUS(status)); - else if (WIFSIGNALED(status)) - ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", tag, - WTERMSIG(status)); - else if (WIFSTOPPED(status)) - ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", tag, - WSTOPSIG(status)); - if (chld_sts != NULL) - *chld_sts = status; - } else - ALOG(LOG_INFO, "logwrapper", "%s wait() failed: %s (%d)", tag, - strerror(errno), errno); + } else if (WIFSIGNALED(status)) { + ALOG(LOG_INFO, "logwrapper", "%s terminated by signal %d", tag, + WTERMSIG(status)); + } else if (WIFSTOPPED(status)) { + ALOG(LOG_INFO, "logwrapper", "%s stopped by signal %d", tag, + WSTOPSIG(status)); + } + if (chld_sts != NULL) + *chld_sts = status; + + return 0; } -void child(int argc, char* argv[]) { +static void child(int argc, char* argv[]) { // create null terminated argv_child array char* argv_child[argc + 1]; memcpy(argv_child, argv, argc * sizeof(char *)); @@ -111,12 +149,13 @@ void child(int argc, char* argv[]) { } } -int logwrap(int argc, char* argv[], int *chld_sts) { +int logwrap(int argc, char* argv[], int *status) { pid_t pid; int parent_ptty; int child_ptty; char *child_devname = NULL; + sigset_t chldset; /* Use ptty instead of socketpair so that STDOUT is not buffered */ parent_ptty = open("/dev/ptmx", O_RDWR); @@ -129,13 +168,19 @@ int logwrap(int argc, char* argv[], int *chld_sts) { return fatal("Problem with /dev/ptmx\n"); } + sigemptyset(&chldset); + sigaddset(&chldset, SIGCHLD); + sigprocmask(SIG_BLOCK, &chldset, NULL); + pid = fork(); if (pid < 0) { close(parent_ptty); + sigprocmask(SIG_UNBLOCK, &chldset, NULL); return fatal("Failed to fork\n"); } else if (pid == 0) { - child_ptty = open(child_devname, O_RDWR); close(parent_ptty); + sigprocmask(SIG_UNBLOCK, &chldset, NULL); + child_ptty = open(child_devname, O_RDWR); if (child_ptty < 0) { return fatal("Problem with child ptty\n"); } @@ -146,16 +191,35 @@ int logwrap(int argc, char* argv[], int *chld_sts) { close(child_ptty); child(argc - 1, &argv[1]); + return fatal("This should never happen\n"); } else { + int rc; + int fd; + + fd = signalfd(-1, &chldset, SFD_NONBLOCK); + if (fd == -1) { + char msg[40]; + + snprintf(msg, sizeof(msg), "signalfd failed: %d\n", errno); + + close(parent_ptty); + sigprocmask(SIG_UNBLOCK, &chldset, NULL); + return fatal(msg); + } + // switch user and group to "log" - // this may fail if we are not root, - // but in that case switching user/group is unnecessary + // this may fail if we are not root, + // but in that case switching user/group is unnecessary setgid(AID_LOG); setuid(AID_LOG); - parent(argv[1], parent_ptty, chld_sts); - } + rc = parent(argv[1], parent_ptty, fd, pid, status); + close(parent_ptty); + close(fd); - return 0; + sigprocmask(SIG_UNBLOCK, &chldset, NULL); + + return rc; + } } -- cgit v1.2.3