You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by br...@apache.org on 2014/05/21 18:50:00 UTC
[1/2] git commit: [TS-2822] Crash in LogBufferIterator::next
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 <br...@apache.org>
Authored: Wed May 21 09:49:13 2014 -0700
Committer: Brian Geffon <br...@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.
+ }
+ } while (total_read < buffer_bytes);
// Possibly skip too old entries (the entire buffer is skipped)
if (header->high_timestamp >= max_age) {
Re: [1/2] git commit: [TS-2822] Crash in LogBufferIterator::next
Posted by James Peach <jp...@apache.org>.
On May 21, 2014, at 9:50 AM, briang@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 <br...@apache.org>
> Authored: Wed May 21 09:49:13 2014 -0700
> Committer: Brian Geffon <br...@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) {
>
[2/2] git commit: [TS-2822] Crash in LogBufferIterator::next
Posted by br...@apache.org.
[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/374355c0
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/374355c0
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/374355c0
Branch: refs/heads/master
Commit: 374355c01caa7e2e88a51def5506abee6209fa51
Parents: 7c314c5
Author: Brian Geffon <br...@apache.org>
Authored: Wed May 21 09:49:37 2014 -0700
Committer: Brian Geffon <br...@apache.org>
Committed: Wed May 21 09:49:37 2014 -0700
----------------------------------------------------------------------
CHANGES | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/374355c0/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 851f232..36fbb50 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 5.0.0
+ *) [TS-2822] Crash in LogBufferIterator::next
+
*) [TS-2705] Make the background fill check more robust.
*) [TS-1411] Seg fault when using %<cquuc>
Re: [1/2] git commit: [TS-2822] Crash in LogBufferIterator::next
Posted by James Peach <jp...@apache.org>.
On May 21, 2014, at 9:50 AM, briang@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 <br...@apache.org>
> Authored: Wed May 21 09:49:13 2014 -0700
> Committer: Brian Geffon <br...@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) {
>