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
This commit is contained in:
Rom Lemarchand 2013-01-09 21:31:25 -08:00
parent 1b325d1968
commit b58a822955
2 changed files with 131 additions and 45 deletions

View file

@ -20,7 +20,29 @@
__BEGIN_DECLS
int logwrap(int argc, char* argv[], int *chld_sts);
/*
* Run a command while logging its stdout and stderr
*
* WARNING: while this function is running it will clear all SIGCHLD handlers
* if you rely on SIGCHLD in the caller there is a chance zombies will be
* created if you're not calling waitpid after calling this. This function will
* log a warning when it clears SIGCHLD for processes other than the child it
* created.
*
* Arguments:
* argc: the number of elements in argv
* argv: an array of strings containing the command to be executed and its
* arguments as separate strings. argv does not need to be
* NULL-terminated
* status: the equivalent child status as populated by wait(status). This
* value is only valid when logwrap successfully completes
*
* Return value:
* 0 when logwrap successfully run the child process and captured its status
* -1 when an internal error occurred
*
*/
int logwrap(int argc, char* argv[], int *status);
__END_DECLS

View file

@ -16,6 +16,9 @@
#include <string.h>
#include <sys/types.h>
#include <sys/signalfd.h>
#include <signal.h>
#include <poll.h>
#include <sys/wait.h>
#include <stdio.h>
#include <stdlib.h>
@ -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;
}
}