diff --git a/liblog/logd_reader.cpp b/liblog/logd_reader.cpp index dcdff055a..619cf8c54 100644 --- a/liblog/logd_reader.cpp +++ b/liblog/logd_reader.cpp @@ -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; } diff --git a/liblog/tests/log_wrap_test.cpp b/liblog/tests/log_wrap_test.cpp index c7dd8e812..e06964f72 100644 --- a/liblog/tests/log_wrap_test.cpp +++ b/liblog/tests/log_wrap_test.cpp @@ -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