On May 21, 2014, at 9:50 AM, bri...@apache.org wrote:
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 2ba2baaa6 -> 374355c01
>
>
> [TS-2822] Crash in LogBufferIterator::next
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7c314c5c
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7c314c5c
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7c314c5c
>
> Branch: refs/heads/master
> Commit: 7c314c5c9bfa347fd583500698c2c586f668f7ea
> Parents: 2ba2baa
> Author: Brian Geffon <bri...@apache.org>
> Authored: Wed May 21 09:49:13 2014 -0700
> Committer: Brian Geffon <bri...@apache.org>
> Committed: Wed May 21 09:49:13 2014 -0700
>
> ----------------------------------------------------------------------
> proxy/logstats.cc | 37 +++++++++++++++++++++++++++++++------
> 1 file changed, 31 insertions(+), 6 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7c314c5c/proxy/logstats.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/logstats.cc b/proxy/logstats.cc
> index 15aeaa2..c6386a6 100644
> --- a/proxy/logstats.cc
> +++ b/proxy/logstats.cc
> @@ -1672,6 +1672,7 @@ parse_log_buff(LogBufferHeader * buf_header, bool
> summary = false)
> int
> process_file(int in_fd, off_t offset, unsigned max_age)
> {
> + ink_release_assert(in_fd < FD_SETSIZE); // If you're exceeding this then
> use CLOSEONEXEC with your opens before FORK/EXEC
> char buffer[MAX_LOGBUFFER_SIZE];
> int nread, buffer_bytes;
>
> @@ -1730,7 +1731,7 @@ process_file(int in_fd, off_t offset, unsigned max_age)
> unsigned second_read_size = sizeof(LogBufferHeader) - first_read_size;
> nread = read(in_fd, &buffer[first_read_size], second_read_size);
> if (!nread || EOF == nread) {
> - Debug("logstats", "Second read of header failed (attemped %d bytes at
> offset %d, got nothing).", second_read_size, first_read_size);
> + Debug("logstats", "Second read of header failed (attemped %d bytes at
> offset %d, got nothing), errno=%d.", second_read_size, first_read_size,
> errno);
> return 1;
> }
>
> @@ -1746,11 +1747,35 @@ process_file(int in_fd, off_t offset, unsigned
> max_age)
> return 1;
> }
>
> - nread = read(in_fd, &buffer[sizeof(LogBufferHeader)], buffer_bytes);
> - if (!nread || EOF == nread) {
> - Debug("logstats", "Failed to read buffer payload [%d bytes]",
> buffer_bytes);
> - return 1;
> - }
> + const int MAX_READ_TRIES = 5;
> + int total_read = 0;
> + int read_tries_remaining = MAX_READ_TRIES; // since the data will be old
> anyway, let's only try a few times.
> + nread = 0;
> + do {
> + nread = read(in_fd, &buffer[sizeof(LogBufferHeader) + total_read],
> buffer_bytes - total_read);
> + if (EOF == nread || !nread) { // just bail on error
> + Debug("logstats", "Read failed while reading log buffer, wanted %d
> bytes, nread=%d, total_read=%d, errno=%d, tries_remaining=%d", buffer_bytes -
> total_read, nread, total_read,
> + errno, read_tries_remaining);
> + return 1;
> + } else {
> + total_read += nread;
> + }
> +
> + if (total_read < buffer_bytes) {
> + if (--read_tries_remaining <= 0) {
> + Debug("logstats_failed_retries", "Unable to read after %d tries,
> total_read=%d, buffer_bytes=%d", MAX_READ_TRIES, total_read, buffer_bytes);
> + return 1;
> + }
> + // let's wait until we get more data on this file descriptor
> + Debug("logstats_partial_read", "Failed to read buffer payload [%d
> bytes], total_read=%d, buffer_bytes=%d, tries_remaining=%d",
> + buffer_bytes - total_read, total_read, buffer_bytes,
> read_tries_remaining);
> + fd_set fds;
> + struct timeval tv = {0, 50*1000}; // wait only up to 50ms
> + FD_ZERO(&fds);
> + FD_SET(in_fd, &fds);
> + select(in_fd + 1, &fds, NULL, NULL, &tv); // we only need to select
> up to in_fd + 1.
Can in_fd ever be a socket?
> + }
> + } while (total_read < buffer_bytes);
>
> // Possibly skip too old entries (the entire buffer is skipped)
> if (header->high_timestamp >= max_age) {
>
