You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2015/01/30 04:09:02 UTC
[1/2] trafficserver git commit: TS-3287: fix various assertion
side-effects in jtest
Repository: trafficserver
Updated Branches:
refs/heads/master 0484b3009 -> 4beccb0e3
TS-3287: fix various assertion side-effects in jtest
Coverity CID #1242345
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4beccb0e
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4beccb0e
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4beccb0e
Branch: refs/heads/master
Commit: 4beccb0e33f3cd49ea1aba7fbf9be078761408fc
Parents: 80323d5
Author: James Peach <jp...@apache.org>
Authored: Mon Jan 12 16:31:51 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Thu Jan 29 19:08:10 2015 -0800
----------------------------------------------------------------------
tools/jtest/jtest.cc | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4beccb0e/tools/jtest/jtest.cc
----------------------------------------------------------------------
diff --git a/tools/jtest/jtest.cc b/tools/jtest/jtest.cc
index ce1b3b2..7ac8dab 100644
--- a/tools/jtest/jtest.cc
+++ b/tools/jtest/jtest.cc
@@ -138,7 +138,7 @@ static int proxy_port = 8080;
static unsigned int proxy_addr = 0;
static unsigned int local_addr = 0;
static char proxy_host[81] = "localhost";
-static char local_host[81];
+static char local_host[255 + 1];
static int verbose = 0;
static int verbose_errors = 1;
static int debug = 0;
@@ -2647,7 +2647,10 @@ UrlHashTable::UrlHashTable()
return;
if (*url_hash_filename) {
- ink_assert((fd = open(url_hash_filename,O_RDWR|O_CREAT, 0644))>0);
+ if ((fd = open(url_hash_filename,O_RDWR|O_CREAT, 0644)) == -1) {
+ panic_perror("failed to open URL Hash file");
+ }
+
len = lseek(fd, 0, SEEK_END);
}
@@ -2677,7 +2680,10 @@ UrlHashTable::UrlHashTable()
}
if (*url_hash_filename) {
- ink_assert( !ftruncate(fd,numbytes) );
+ if (ftruncate(fd, numbytes) == -1) {
+ panic_perror("unable to truncate URL Hash file");
+ }
+
bytes = (unsigned char *)
mmap(NULL,numbytes,PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
if (bytes == (unsigned char*)MAP_FAILED || !bytes)
@@ -2691,8 +2697,8 @@ UrlHashTable::UrlHashTable()
UrlHashTable::~UrlHashTable()
{
- ink_assert(!munmap((char*)bytes, numbytes));
- ink_assert(!close(fd));
+ if (bytes) { munmap((char*)bytes, numbytes); }
+ if (fd != -1) { close(fd); }
} // UrlHashTable::~UrlHashTable
static int seen_it(char * url) {
@@ -2884,8 +2890,12 @@ int main(int argc __attribute__((unused)), char *argv[])
urls_mode = n_file_arguments || *urls_file;
nclients = client_rate? 0 : nclients;
- if (!local_host[0])
- ink_assert(!gethostname(local_host,80));
+ if (!local_host[0]) {
+ if (gethostname(local_host, sizeof(local_host)) != 0) {
+ panic_perror("gethostname failed");
+ }
+ }
+
local_addr = get_addr(local_host);
if (!proxy_host[0])
strcpy(proxy_host, local_host);
[2/2] trafficserver git commit: TS-3287: improve select descriptor
handling in libmgmt
Posted by jp...@apache.org.
TS-3287: improve select descriptor handling in libmgmt
Fix misue of FD_SETSIZE im mgmt_write_timeout and add the corresponding
check in mgmt_read_timeout. Remove unnecessary fd_set manipulation
in preference of the mgmt timeout wrappers.
Coverity CID #1237317
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/80323d52
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/80323d52
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/80323d52
Branch: refs/heads/master
Commit: 80323d52556878b4f1a7a146a23e812a6f68759e
Parents: 0484b30
Author: James Peach <jp...@apache.org>
Authored: Mon Jan 12 15:22:42 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Thu Jan 29 19:08:10 2015 -0800
----------------------------------------------------------------------
mgmt/ProcessManager.cc | 15 ++---------
mgmt/cluster/ClusterCom.cc | 55 ++++++++++++++---------------------------
mgmt/utils/MgmtSocket.cc | 4 +--
3 files changed, 23 insertions(+), 51 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/80323d52/mgmt/ProcessManager.cc
----------------------------------------------------------------------
diff --git a/mgmt/ProcessManager.cc b/mgmt/ProcessManager.cc
index 998ccef..2f37fae 100644
--- a/mgmt/ProcessManager.cc
+++ b/mgmt/ProcessManager.cc
@@ -232,28 +232,17 @@ void
ProcessManager::pollLMConnection()
{
int res;
- struct timeval poll_timeout;
MgmtMessageHdr mh_hdr;
MgmtMessageHdr *mh_full;
char *data_raw;
- int num;
- fd_set fdlist;
-
while (1) {
+ int num;
- // poll only
- poll_timeout.tv_sec = 0;
- poll_timeout.tv_usec = 1000;
-
- FD_ZERO(&fdlist);
- FD_SET(local_manager_sockfd, &fdlist);
- num = mgmt_select(FD_SETSIZE, &fdlist, NULL, NULL, &poll_timeout);
+ num = mgmt_read_timeout(local_manager_sockfd, 1 /* sec */, 0 /* usec */);
if (num == 0) { /* Have nothing */
-
break;
-
} else if (num > 0) { /* We have a message */
if ((res = mgmt_read_pipe(local_manager_sockfd, (char *) &mh_hdr, sizeof(MgmtMessageHdr))) > 0) {
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/80323d52/mgmt/cluster/ClusterCom.cc
----------------------------------------------------------------------
diff --git a/mgmt/cluster/ClusterCom.cc b/mgmt/cluster/ClusterCom.cc
index c3063ce..a5a93b0 100644
--- a/mgmt/cluster/ClusterCom.cc
+++ b/mgmt/cluster/ClusterCom.cc
@@ -55,7 +55,6 @@ static void *
drainIncomingChannel_broadcast(void *arg)
{
char message[61440];
- fd_set fdlist;
ClusterCom * ccom = (ClusterCom *)arg;
time_t t;
@@ -70,26 +69,21 @@ drainIncomingChannel_broadcast(void *arg)
lmgmt->syslogThrInit();
for (;;) { /* Loop draining mgmt network channels */
- // linux: set tv.tv_set in select() loop, since linux's select()
- // will update tv with the amount of time not slept (most other
- // implementations do not do this)
- tv.tv_sec = ccom->mc_poll_timeout; // interface not-responding timeout
- tv.tv_usec = 0;
+ int nevents = 0;
- memset(message, 0, 61440);
- FD_ZERO(&fdlist);
-
- if (ccom->cluster_type != NO_CLUSTER) {
- if (ccom->receive_fd > 0) {
- FD_SET(ccom->receive_fd, &fdlist); /* Multicast fd */
- }
+ // It's not clear whether this can happen, but historically, this code was written as if it
+ // could. A hacky little sleep here will prevent this thread spinning on the read timeout.
+ if (ccom->cluster_type == NO_CLUSTER || ccom->receive_fd == ts::NO_FD) {
+ mgmt_sleep_sec(1);
}
- mgmt_select(FD_SETSIZE, &fdlist, NULL, NULL, &tv);
+ ink_zero(message);
if (ccom->cluster_type != NO_CLUSTER) {
- // Multicast timeout considerations
- if ((ccom->receive_fd < 0) || !FD_ISSET(ccom->receive_fd, &fdlist)) {
+ nevents = mgmt_read_timeout(ccom->receive_fd, ccom->mc_poll_timeout /* secs */, 0 /* usecs */);
+ if (nevents > 0) {
+ last_multicast_receive_time = time(NULL); // valid multicast msg
+ } else {
t = time(NULL);
if ((t - last_multicast_receive_time) > (tv.tv_sec - 1)) {
// Timeout on multicast receive channel, reset channel.
@@ -104,16 +98,14 @@ drainIncomingChannel_broadcast(void *arg)
}
last_multicast_receive_time = t; // next action at next interval
}
- } else {
- last_multicast_receive_time = time(NULL); // valid multicast msg
}
}
/* Broadcast message */
if (ccom->cluster_type != NO_CLUSTER &&
ccom->receive_fd > 0 &&
- FD_ISSET(ccom->receive_fd, &fdlist) &&
- (ccom->receiveIncomingMessage(message, 61440) > 0)) {
+ nevents > 0 &&
+ (ccom->receiveIncomingMessage(message, sizeof(message)) > 0)) {
ccom->handleMultiCastMessage(message);
}
}
@@ -127,11 +119,10 @@ drainIncomingChannel_broadcast(void *arg)
* continuous draining of the network. It drains and handles requests made on
* the reliable and multicast channels between all the peers.
*/
-void *
+static void *
drainIncomingChannel(void *arg)
{
char message[61440];
- fd_set fdlist;
ClusterCom * ccom = (ClusterCom *)arg;
struct sockaddr_in cli_addr;
@@ -162,7 +153,6 @@ drainIncomingChannel(void *arg)
// to reopen the channel (e.g. opening the socket would fail if the
// interface was down). In this case, the ccom->receive_fd is set
// to '-1' and the open is retried until it succeeds.
- struct timeval tv;
/* Avert race condition, thread spun during constructor */
while (lmgmt->ccom != ccom || !lmgmt->ccom->init) {
@@ -172,22 +162,15 @@ drainIncomingChannel(void *arg)
lmgmt->syslogThrInit();
for (;;) { /* Loop draining mgmt network channels */
- // linux: set tv.tv_set in select() loop, since linux's select()
- // will update tv with the amount of time not slept (most other
- // implementations do not do this)
- tv.tv_sec = ccom->mc_poll_timeout; // interface not-responding timeout
- tv.tv_usec = 0;
+ ink_zero(message);
- memset(message, 0, 61440);
- FD_ZERO(&fdlist);
-
- if (ccom->cluster_type != NO_CLUSTER) {
- FD_SET(ccom->reliable_server_fd, &fdlist); /* TCP Server fd */
+ // It's not clear whether this can happen, but historically, this code was written as if it
+ // could. A hacky little sleep here will prevent this thread spinning on the read timeout.
+ if (ccom->cluster_type == NO_CLUSTER || ccom->reliable_server_fd == ts::NO_FD) {
+ mgmt_sleep_sec(1);
}
- mgmt_select(FD_SETSIZE, &fdlist, NULL, NULL, &tv);
-
- if (FD_ISSET(ccom->reliable_server_fd, &fdlist)) {
+ if (mgmt_read_timeout(ccom->reliable_server_fd, ccom->mc_poll_timeout /* secs */, 0 /* usecs */) > 0) {
/* Reliable(TCP) request */
int clilen = sizeof(cli_addr);
int req_fd = mgmt_accept(ccom->reliable_server_fd, (struct sockaddr *) &cli_addr, &clilen);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/80323d52/mgmt/utils/MgmtSocket.cc
----------------------------------------------------------------------
diff --git a/mgmt/utils/MgmtSocket.cc b/mgmt/utils/MgmtSocket.cc
index 96372e9..fa73aaa 100644
--- a/mgmt/utils/MgmtSocket.cc
+++ b/mgmt/utils/MgmtSocket.cc
@@ -214,7 +214,7 @@ mgmt_write_timeout(int fd, int sec, int usec)
timeout.tv_sec = sec;
timeout.tv_usec = usec;
- if (fd < 0 || fd > FD_SETSIZE) {
+ if (fd < 0 || fd >= FD_SETSIZE) {
errno = EBADF;
return -1;
}
@@ -252,7 +252,7 @@ mgmt_read_timeout(int fd, int sec, int usec)
timeout.tv_sec = sec;
timeout.tv_usec = usec;
- if (fd < 0) {
+ if (fd < 0 || fd >= FD_SETSIZE) {
errno = EBADF;
return -1;
}