You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ma...@apache.org on 2008/11/19 21:47:22 UTC
svn commit: r719063 - in /hadoop/zookeeper/trunk: CHANGES.txt
src/c/src/zookeeper.c src/c/tests/TestZookeeperInit.cc
Author: mahadev
Date: Wed Nov 19 12:47:22 2008
New Revision: 719063
URL: http://svn.apache.org/viewvc?rev=719063&view=rev
Log:
ZOOKEEPER-208. Zookeeper C client uses API that are not thread safe, causing crashes when multiple instances are active. (austin shoemaker, chris daroch and ben reed via mahadev)
Modified:
hadoop/zookeeper/trunk/CHANGES.txt
hadoop/zookeeper/trunk/src/c/src/zookeeper.c
hadoop/zookeeper/trunk/src/c/tests/TestZookeeperInit.cc
Modified: hadoop/zookeeper/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/CHANGES.txt?rev=719063&r1=719062&r2=719063&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/CHANGES.txt (original)
+++ hadoop/zookeeper/trunk/CHANGES.txt Wed Nov 19 12:47:22 2008
@@ -32,6 +32,10 @@
ZOOKEEPER-204. SetWatches needs to be the first message after auth messages
to the server (ben via mahadev)
+
+ ZOOKEEPER-208. Zookeeper C client uses API that are not thread safe,
+causing crashes when multiple instances are active. (austin shoemaker, chris
+daroch and ben reed via mahadev)
Release 3.0.0 - 2008-10-21
Modified: hadoop/zookeeper/trunk/src/c/src/zookeeper.c
URL: http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/c/src/zookeeper.c?rev=719063&r1=719062&r2=719063&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/src/c/src/zookeeper.c (original)
+++ hadoop/zookeeper/trunk/src/c/src/zookeeper.c Wed Nov 19 12:47:22 2008
@@ -281,13 +281,11 @@
*/
int getaddrs(zhandle_t *zh)
{
- struct hostent *he;
+ struct addrinfo hints, *res, *res0;
struct sockaddr *addr;
- struct sockaddr_in *addr4;
- struct sockaddr_in6 *addr6;
- char **ptr;
char *hosts = strdup(zh->hostname);
char *host;
+ char *strtok_last;
int i;
int rc;
int alen = 0; /* the allocated length of the addrs array */
@@ -303,7 +301,7 @@
return ZSYSTEMERROR;
}
zh->addrs = 0;
- host=strtok(hosts, ",");
+ host=strtok_r(hosts, ",", &strtok_last);
while(host) {
char *port_spec = strchr(host, ':');
char *end_port_spec;
@@ -323,16 +321,21 @@
rc=ZBADARGUMENTS;
goto fail;
}
- he = gethostbyname(host);
- if (!he) {
- LOG_ERROR(("could not resolve %s", host));
- errno=EINVAL;
- rc=ZBADARGUMENTS;
+
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_flags = AI_ADDRCONFIG;
+ hints.ai_family = AF_UNSPEC;
+ hints.ai_socktype = SOCK_STREAM;
+ hints.ai_protocol = IPPROTO_TCP;
+
+ if (getaddrinfo(host, port_spec, &hints, &res0) != 0) {
+ LOG_ERROR(("getaddrinfo: %s\n", strerror(errno)));
+ rc=ZSYSTEMERROR;
goto fail;
}
-
- /* Setup the address array */
- for(ptr = he->h_addr_list;*ptr != 0; ptr++) {
+
+ for (res = res0; res; res = res->ai_next) {
+ // Expand address list if needed
if (zh->addrs_count == alen) {
void *tmpaddr;
alen += 16;
@@ -345,31 +348,30 @@
}
zh->addrs=tmpaddr;
}
+
+ // Copy addrinfo into address list
addr = &zh->addrs[zh->addrs_count];
- addr4 = (struct sockaddr_in*)addr;
- addr6 = (struct sockaddr_in6*)addr;
- addr->sa_family = he->h_addrtype;
- if (addr->sa_family == AF_INET) {
- addr4->sin_port = htons(port);
- memset(&addr4->sin_zero, 0, sizeof(addr4->sin_zero));
- memcpy(&addr4->sin_addr, *ptr, he->h_length);
- zh->addrs_count++;
+ switch (res->ai_family) {
+ case AF_INET:
#if defined(AF_INET6)
- } else if (addr->sa_family == AF_INET6) {
- addr6->sin6_port = htons(port);
- addr6->sin6_scope_id = 0;
- addr6->sin6_flowinfo = 0;
- memcpy(&addr6->sin6_addr, *ptr, he->h_length);
- zh->addrs_count++;
+ case AF_INET6:
#endif
- } else {
- LOG_WARN(("skipping unknown address family %x for %s",
- addr->sa_family, zh->hostname));
+ memcpy(addr, res->ai_addr, res->ai_addrlen);
+ ++zh->addrs_count;
+ break;
+ default:
+ LOG_WARN(("skipping unknown address family %x for %s",
+ res->ai_family, zh->hostname));
+ break;
}
}
- host = strtok(0, ",");
+
+ freeaddrinfo(res0);
+
+ host = strtok_r(0, ",", &strtok_last);
}
free(hosts);
+
if(!disable_conn_permute){
setup_random();
/* Permute */
Modified: hadoop/zookeeper/trunk/src/c/tests/TestZookeeperInit.cc
URL: http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/c/tests/TestZookeeperInit.cc?rev=719063&r1=719062&r2=719063&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/src/c/tests/TestZookeeperInit.cc (original)
+++ hadoop/zookeeper/trunk/src/c/tests/TestZookeeperInit.cc Wed Nov 19 12:47:22 2008
@@ -121,14 +121,11 @@
}
void testAddressResolution()
{
- const char EXPECTED_IPS[][4]={{127,0,0,1},{127,0,0,2},{127,0,0,3}};
+ const char EXPECTED_IPS[][4]={{127,0,0,1}};
const int EXPECTED_ADDRS_COUNT =COUNTOF(EXPECTED_IPS);
- Mock_gethostbyname mock;
- mock.addHostEntry("somehostname").addAddress(EXPECTED_IPS[0]).
- addAddress(EXPECTED_IPS[1]).addAddress(EXPECTED_IPS[2]);
zoo_deterministic_conn_order(1);
- zh=zookeeper_init("host:2121",0,10000,0,0,0);
+ zh=zookeeper_init("127.0.0.1:2121",0,10000,0,0,0);
CPPUNIT_ASSERT(zh!=0);
CPPUNIT_ASSERT_EQUAL(EXPECTED_ADDRS_COUNT,zh->addrs_count);
@@ -140,15 +137,9 @@
}
void testMultipleAddressResolution()
{
- const string EXPECTED_HOST("host1:2121,host2:3434");
- const char EXPECTED_IPS[][4]={{127,0,0,1},{127,0,0,2},{127,0,0,3},
- {126,1,1,1},{126,2,2,2}};
+ const string EXPECTED_HOST("127.0.0.1:2121,127.0.0.2:3434");
+ const char EXPECTED_IPS[][4]={{127,0,0,1},{127,0,0,2}};
const int EXPECTED_ADDRS_COUNT =COUNTOF(EXPECTED_IPS);
- Mock_gethostbyname mock;
- mock.addHostEntry("somehost1").addAddress(EXPECTED_IPS[0]).
- addAddress(EXPECTED_IPS[1]).addAddress(EXPECTED_IPS[2]);
- mock.addHostEntry("somehost2").addAddress(EXPECTED_IPS[3]).
- addAddress(EXPECTED_IPS[4]);
zoo_deterministic_conn_order(1);
zh=zookeeper_init(EXPECTED_HOST.c_str(),0,1000,0,0,0);
@@ -159,7 +150,7 @@
for(int i=0;i<zh->addrs_count;i++){
sockaddr_in* addr=(struct sockaddr_in*)&zh->addrs[i];
CPPUNIT_ASSERT(memcmp(EXPECTED_IPS[i],&addr->sin_addr,sizeof(addr->sin_addr))==0);
- if(i<3)
+ if(i<1)
CPPUNIT_ASSERT_EQUAL(2121,(int)ntohs(addr->sin_port));
else
CPPUNIT_ASSERT_EQUAL(3434,(int)ntohs(addr->sin_port));
@@ -208,14 +199,15 @@
}
void testNonexistentHost()
{
- const string EXPECTED_HOST("host1:1111");
- MockFailed_gethostbyname mock;
+ const string EXPECTED_HOST("host1.blabadibla.bla.:1111");
zh=zookeeper_init(EXPECTED_HOST.c_str(),0,0,0,0,0);
CPPUNIT_ASSERT(zh==0);
- CPPUNIT_ASSERT_EQUAL(EINVAL,errno);
- CPPUNIT_ASSERT_EQUAL(HOST_NOT_FOUND,h_errno);
+ //With the switch to thread safe getaddrinfo, we don't get
+ //these global variables
+ //CPPUNIT_ASSERT_EQUAL(EINVAL,errno);
+ //CPPUNIT_ASSERT_EQUAL(HOST_NOT_FOUND,h_errno);
}
void testOutOfMemory_init()
{
@@ -232,30 +224,17 @@
Mock_realloc reallocMock;
reallocMock.callsBeforeFailure=0; // fail on first call to realloc
- Mock_gethostbyname gethostbynameMock;
- gethostbynameMock.addHostEntry("ahost").addAddress("\1\1\1\1");
-
- zh=zookeeper_init("ahost:123",0,0,0,0,0);
+ zh=zookeeper_init("127.0.0.1:123",0,0,0,0,0);
CPPUNIT_ASSERT(zh==0);
CPPUNIT_ASSERT_EQUAL(ENOMEM,errno);
}
void testOutOfMemory_getaddrs2()
{
- const char ADDR[]="\1\1\1\1";
Mock_realloc reallocMock;
reallocMock.callsBeforeFailure=1; // fail on the second call to realloc
- Mock_gethostbyname gethostbynameMock;
- // need >16 IPs to get realloc called the second time
- gethostbynameMock.addHostEntry("ahost").
- addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).
- addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).
- addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).
- addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).addAddress(ADDR).
- addAddress(ADDR);
-
- zh=zookeeper_init("ahost:123",0,0,0,0,0);
+ zh=zookeeper_init("127.0.0.1:123,127.0.0.2:123,127.0.0.3:123,127.0.0.4:123,127.0.0.5:123,127.0.0.6:123,127.0.0.7:123,127.0.0.8:123,127.0.0.9:123,127.0.0.10:123,127.0.0.11:123,127.0.0.12:123,127.0.0.13:123,127.0.0.14:123,127.0.0.15:123,127.0.0.16:123,127.0.0.17:123",0,0,0,0,0);
CPPUNIT_ASSERT(zh==0);
CPPUNIT_ASSERT_EQUAL(ENOMEM,errno);
@@ -265,16 +244,11 @@
const char EXPECTED[][5]={"\0\0\0\0","\1\1\1\1","\2\2\2\2","\3\3\3\3"};
const int EXPECTED_ADDR_COUNT=COUNTOF(EXPECTED);
- Mock_gethostbyname gethostbynameMock;
- gethostbynameMock.addHostEntry("ahost").
- addAddress(EXPECTED[0]).addAddress(EXPECTED[1]).
- addAddress(EXPECTED[2]).addAddress(EXPECTED[3]);
-
const int RAND_SEQ[]={0,1,2,3,1,3,2,0,-1};
const int RAND_SIZE=COUNTOF(RAND_SEQ);
Mock_random randomMock;
randomMock.randomReturns.assign(RAND_SEQ,RAND_SEQ+RAND_SIZE-1);
- zh=zookeeper_init("ahost:123",0,1000,0,0,0);
+ zh=zookeeper_init("0.0.0.0:123,1.1.1.1:123,2.2.2.2:123,3.3.3.3:123",0,1000,0,0,0);
CPPUNIT_ASSERT(zh!=0);
CPPUNIT_ASSERT_EQUAL(EXPECTED_ADDR_COUNT,zh->addrs_count);