Merge "liblog: remove alarm in logd_reader.cpp"

This commit is contained in:
Tom Cherry 2019-10-09 14:48:38 +00:00 committed by Gerrit Code Review
commit e3fc9ac7e3
2 changed files with 22 additions and 107 deletions

View file

@ -316,16 +316,11 @@ static ssize_t logdSetPrune(struct android_log_logger_list* logger_list __unused
return check_log_success(buf, send_log_msg(NULL, NULL, buf, len));
}
static void caught_signal(int signum __unused) {}
static int logdOpen(struct android_log_logger_list* logger_list,
struct android_log_transport_context* transp) {
struct android_log_logger* logger;
struct sigaction ignore;
struct sigaction old_sigaction;
unsigned int old_alarm = 0;
char buffer[256], *cp, c;
int e, ret, remaining, sock;
int ret, remaining, sock;
if (!logger_list) {
return -EINVAL;
@ -387,29 +382,13 @@ static int logdOpen(struct android_log_logger_list* logger_list,
cp += ret;
}
if (logger_list->mode & ANDROID_LOG_NONBLOCK) {
/* Deal with an unresponsive logd */
memset(&ignore, 0, sizeof(ignore));
ignore.sa_handler = caught_signal;
sigemptyset(&ignore.sa_mask);
/* particularily useful if tombstone is reporting for logd */
sigaction(SIGALRM, &ignore, &old_sigaction);
old_alarm = alarm(30);
}
ret = write(sock, buffer, cp - buffer);
e = errno;
if (logger_list->mode & ANDROID_LOG_NONBLOCK) {
if (e == EINTR) {
e = ETIMEDOUT;
}
alarm(old_alarm);
sigaction(SIGALRM, &old_sigaction, NULL);
}
ret = TEMP_FAILURE_RETRY(write(sock, buffer, cp - buffer));
int write_errno = errno;
if (ret <= 0) {
close(sock);
if ((ret == -1) && e) {
return -e;
if (ret == -1) {
return -write_errno;
}
if (ret == 0) {
return -EIO;
@ -427,52 +406,21 @@ static int logdOpen(struct android_log_logger_list* logger_list,
/* Read from the selected logs */
static int logdRead(struct android_log_logger_list* logger_list,
struct android_log_transport_context* transp, struct log_msg* log_msg) {
int ret, e;
struct sigaction ignore;
struct sigaction old_sigaction;
unsigned int old_alarm = 0;
ret = logdOpen(logger_list, transp);
int ret = logdOpen(logger_list, transp);
if (ret < 0) {
return ret;
}
memset(log_msg, 0, sizeof(*log_msg));
unsigned int new_alarm = 0;
if (logger_list->mode & ANDROID_LOG_NONBLOCK) {
if ((logger_list->mode & ANDROID_LOG_WRAP) &&
(logger_list->start.tv_sec || logger_list->start.tv_nsec)) {
/* b/64143705 */
new_alarm = (ANDROID_LOG_WRAP_DEFAULT_TIMEOUT * 11) / 10 + 10;
logger_list->mode &= ~ANDROID_LOG_WRAP;
} else {
new_alarm = 30;
}
memset(&ignore, 0, sizeof(ignore));
ignore.sa_handler = caught_signal;
sigemptyset(&ignore.sa_mask);
/* particularily useful if tombstone is reporting for logd */
sigaction(SIGALRM, &ignore, &old_sigaction);
old_alarm = alarm(new_alarm);
}
/* NOTE: SOCK_SEQPACKET guarantees we read exactly one full entry */
ret = recv(ret, log_msg, LOGGER_ENTRY_MAX_LEN, 0);
e = errno;
if (new_alarm) {
if ((ret == 0) || (e == EINTR)) {
e = EAGAIN;
ret = -1;
}
alarm(old_alarm);
sigaction(SIGALRM, &old_sigaction, NULL);
ret = TEMP_FAILURE_RETRY(recv(ret, log_msg, LOGGER_ENTRY_MAX_LEN, 0));
if ((logger_list->mode & ANDROID_LOG_NONBLOCK) && ret == 0) {
return -EAGAIN;
}
if ((ret == -1) && e) {
return -e;
if (ret == -1) {
return -errno;
}
return ret;
}

View file

@ -58,60 +58,27 @@ static void read_with_wrap() {
android_logger_list_close(logger_list);
}
static void caught_signal(int /* signum */) {
}
#endif
// b/64143705 confirm fixed
TEST(liblog, wrap_mode_blocks) {
#ifdef __ANDROID__
// The read call is expected to take up to 2 hours in the happy case. There was a previous bug
// where it would take only 30 seconds due to an alarm() in logd_reader.cpp. That alarm has been
// removed, so we check here that the read call blocks for a reasonable amount of time (5s).
struct sigaction ignore = {.sa_handler = [](int) { _exit(0); }};
struct sigaction old_sigaction;
sigaction(SIGALRM, &ignore, &old_sigaction);
alarm(5);
android::base::Timer timer;
read_with_wrap();
// The read call is expected to take up to 2 hours in the happy case.
// We only want to make sure it waits for longer than 30s, but we can't
// use an alarm as the implementation uses it. So we run the test in
// a separate process.
pid_t pid = fork();
if (pid == 0) {
// child
read_with_wrap();
_exit(0);
}
struct sigaction ignore, old_sigaction;
memset(&ignore, 0, sizeof(ignore));
ignore.sa_handler = caught_signal;
sigemptyset(&ignore.sa_mask);
sigaction(SIGALRM, &ignore, &old_sigaction);
alarm(45);
bool killed = false;
for (;;) {
siginfo_t info = {};
// This wait will succeed if the child exits, or fail with EINTR if the
// alarm goes off first - a loose approximation to a timed wait.
int ret = waitid(P_PID, pid, &info, WEXITED);
if (ret >= 0 || errno != EINTR) {
EXPECT_EQ(ret, 0);
if (!killed) {
EXPECT_EQ(info.si_status, 0);
}
break;
}
unsigned int alarm_left = alarm(0);
if (alarm_left > 0) {
alarm(alarm_left);
} else {
kill(pid, SIGTERM);
killed = true;
}
}
FAIL() << "read_with_wrap() should not return before the alarm is triggered.";
alarm(0);
EXPECT_GT(timer.duration(), std::chrono::seconds(40));
sigaction(SIGALRM, &old_sigaction, nullptr);
#else
GTEST_LOG_(INFO) << "This test does nothing.\n";
#endif