[Zookeeper-user] Data races in ZooKeeper C client

[Austin S. <au...@co...>]

We were debugging a crash when using the C client on multiple threads  
and isolated the crash to gethostbyname, which is not reentrant.  
Replacing it with the newer getaddrinfo resolved the problem. The same  
function was also using strtok instead of the thread-safe strtok_r.  
The patch below incorporates both fixes.

There may be other non-threadsafe calls that we haven't discovered-  
let us know if you find any.

Austin


Patch for zookeeper-c-client-2.2.1/src/zookeeper.c (2008-06-09 on  
SF.net)

241c241
<     struct hostent *he;
---
 > 	struct addrinfo hints, *res, *res0;
243,245d242
<     struct sockaddr_in *addr4;
<     struct sockaddr_in6 *addr6;
<     char **ptr;
247a245
 > 	char *strtok_last;
263c261
<     host=strtok(hosts, ",");
---
 >     host=strtok_r(hosts, ",", &strtok_last);
283,294c281,297
<         he = gethostbyname(host);
<         if (!he) {
<             LOG_ERROR(("could not resolve %s", host));
<             errno=EINVAL;
<             rc=ZBADARGUMENTS;
<             goto fail;
<         }
<
<         /* Setup the address array */
<         for(ptr = he->h_addr_list;*ptr != 0; ptr++) {
<             if (zh->addrs_count == alen) {
<                 void *tmpaddr;
---
 > 		
 > 		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;
 > 		}
 > 		
 > 		for (res = res0; res; res = res->ai_next) {
 > 			// Expand address list if needed
 > 			if (zh->addrs_count == alen) {
 > 				void *tmpaddr;
304,313c307,312
<             }
<             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++;
---
 > 			}
 > 			
 > 			// Copy addrinfo into address list
 > 			addr = &zh->addrs[zh->addrs_count];
 > 			switch (res->ai_family) {
 > 			case AF_INET:
315,320c314
<             } 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:
322,327c316,328
<             } else {
<                 LOG_WARN(("skipping unknown address family %x for %s",
<                         addr->sa_family, zh->hostname));
<             }
<         }
<         host = strtok(0, ",");
---
 > 				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;
 > 			}
 > 		}
 > 		
 > 		freeaddrinfo(res0);
 >
 > 		host = strtok_r(0, ",", &strtok_last);
329a331
 >