Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 07 Feb 2004 17:56:41 -0500
From:      Brian Fundakowski Feldman <green@FreeBSD.org>
To:        hackers@FreeBSD.org
Subject:   mostly-reentrant resolver/getaddrinfo(3)
Message-ID:  <200402072256.i17Muf1S099944@green.bikeshed.org>

next in thread | raw e-mail | index | archive | help
I spent a bit of time a few days ago making the resolver and 
getaddrinfo(3)'s DNS functions a little more reentrant than before, with the 
results that Mozilla is behaving much nicer than normal -- if one thread 
("view" or tab in each window, for example, but I forget the Mozilla 
terminology) is waiting forever for a DNS query, the others still continue 
on using their own resolver private data.  I'm pretty sure that the teardown 
functions still need to be made to close the resolver private data correctly 
(i.e. the "s" variable), but it's close enough that it seems to work.  No 
attention has yet been paid to h_errno because it's really an obsolescent 
interface that provides no hard and fast guarantees in a threaded 
environment, according to POSIX.

Comments?  Anyone want to help finish it as a stopgap measure before the 
resolver gets replaced with one from BIND or something?  (By stopgap of 
course I mean we'll end up continuing to use it for another decade.)

Index: include/reentrant.h
===================================================================
RCS file: /usr/ncvs/src/lib/libc/include/reentrant.h,v
retrieving revision 1.2
diff -u -u -r1.2 reentrant.h
--- include/reentrant.h	1 Nov 2002 09:37:17 -0000	1.2
+++ include/reentrant.h	6 Feb 2004 05:31:57 -0000
@@ -123,6 +123,7 @@
 				_pthread_rwlock_unlock(l)
 
 #define thr_keycreate(k, d)	_pthread_key_create(k, d)
+#define thr_keydelete(k)	_pthread_key_delete(k)
 #define thr_setspecific(k, p)	_pthread_setspecific(k, p)
 #define thr_getspecific(k)	_pthread_getspecific(k)
 #define thr_sigsetmask(f, n, o)	_pthread_sigmask(f, n, o)
Index: net/getaddrinfo.c
===================================================================
RCS file: /usr/ncvs/src/lib/libc/net/getaddrinfo.c,v
retrieving revision 1.48
diff -u -u -r1.48 getaddrinfo.c
--- net/getaddrinfo.c	30 Oct 2003 17:36:53 -0000	1.48
+++ net/getaddrinfo.c	6 Feb 2004 06:20:23 -0000
@@ -1511,6 +1511,7 @@
 		return 0;
 	}
 
+	THREAD_UNLOCK();
 	switch (_nsdispatch(&result, dtab, NSDB_HOSTS, "getaddrinfo",
 			default_dns_files, hostname, pai)) {
 	case NS_TRYAGAIN:
@@ -1524,20 +1525,20 @@
 		goto free;
 	case NS_SUCCESS:
 		error = 0;
+		THREAD_LOCK();
 		for (cur = result; cur; cur = cur->ai_next) {
 			GET_PORT(cur, servname);
 			/* canonname should be filled already */
 		}
+		THREAD_UNLOCK();
 		break;
 	}
-	THREAD_UNLOCK();
 
 	*res = result;
 
 	return 0;
 
 free:
-	THREAD_UNLOCK();
 	if (result)
 		freeaddrinfo(result);
 	return error;
@@ -2037,6 +2038,7 @@
 	memset(&sentinel, 0, sizeof(sentinel));
 	cur = &sentinel;
 
+	THREAD_LOCK();
 	_sethtent();
 	while ((p = _gethtent(name, pai)) != NULL) {
 		cur->ai_next = p;
@@ -2044,6 +2046,7 @@
 			cur = cur->ai_next;
 	}
 	_endhtent();
+	THREAD_UNLOCK();
 
 	*((struct addrinfo **)rv) = sentinel.ai_next;
 	if (sentinel.ai_next == NULL)
@@ -2152,9 +2155,12 @@
 	memset(&sentinel, 0, sizeof(sentinel));
 	cur = &sentinel;
 
+	THREAD_LOCK();
 	if (!__ypdomain) {
-		if (_yp_check(&__ypdomain) == 0)
+		if (_yp_check(&__ypdomain) == 0) {
+			THREAD_UNLOCK();
 			return NS_UNAVAIL;
+		}
 	}
 	if (__ypcurrent)
 		free(__ypcurrent);
@@ -2189,6 +2195,7 @@
 				cur = cur->ai_next;
 		}
 	}
+	THREAD_UNLOCK();
 
 	if (sentinel.ai_next == NULL) {
 		h_errno = HOST_NOT_FOUND;
Index: net/res_init.c
===================================================================
RCS file: /usr/ncvs/src/lib/libc/net/res_init.c,v
retrieving revision 1.31
diff -u -u -r1.31 res_init.c
--- net/res_init.c	7 Dec 2003 12:32:24 -0000	1.31
+++ net/res_init.c	6 Feb 2004 14:59:54 -0000
@@ -91,7 +91,11 @@
 #include <unistd.h>
 #include <netdb.h>
 
+#include "namespace.h"
+#include "reentrant.h"
+#include "un-namespace.h"
 #include "res_config.h"
+#include "res_send_private.h"
 
 static void res_setoptions(char *, char *);
 
@@ -106,18 +110,6 @@
 #endif
 
 /*
- * Resolver state default settings.
- */
-
-struct __res_state _res
-# if defined(__BIND_RES_TEXT)
-	= { RES_TIMEOUT, }	/* Motorola, et al. */
-# endif
-	;
-
-struct __res_state_ext _res_ext;
-
-/*
  * Set up default settings.  If the configuration file exist, the values
  * there will have precedence.  Otherwise, the server address is set to
  * INADDR_ANY and the default domain name comes from the gethostname().
@@ -142,6 +134,7 @@
 res_init()
 {
 	FILE *fp;
+	struct res_send_private *rsp;
 	char *cp, **pp;
 	int n;
 	char buf[MAXDNAME];
@@ -157,6 +150,19 @@
 #endif
 
 	/*
+	 * If _res.options (really, ___res()->options) has RES_BOGUS set,
+	 * the allocation for thread-specific data has failed.
+	 */
+	if (_res.options & RES_BOGUS)
+		return (-1);
+	rsp = ___res_send_private();
+	rsp->s = -1;
+	rsp->connected = 0;
+	rsp->vc = 0;
+	rsp->af = 0;
+	rsp->Qhook = NULL;
+	rsp->Rhook = NULL;
+	/*
 	 * These three fields used to be statically initialized.  This made
 	 * it hard to use this code in a shared library.  It is necessary,
 	 * now that we're doing dynamic initialization here, that we preserve
@@ -595,6 +601,85 @@
 
 	gettimeofday(&now, NULL);
 	return (0xffff & (now.tv_sec ^ now.tv_usec ^ getpid()));
+}
+
+/*
+ * Resolver state bogus default settings.
+ */
+
+#undef _res
+#undef _res_ext
+struct __res_state _res = { RES_BOGUS };
+struct __res_state_ext _res_ext;
+
+static thread_key_t res_key, res_ext_key;
+static mutex_t res_init_mutex = MUTEX_INITIALIZER;
+static int res_keys_inited = 0;
+
+struct __res_state *
+___res(void)
+{
+	struct __res_state *myres;
+
+	if (res_keys_inited == 0) {
+		mutex_lock(&res_init_mutex);
+		if (res_keys_inited == 0) {
+			if (thr_keycreate(&res_key, free) != 0)
+				goto failed;
+			if (thr_keycreate(&res_ext_key, free) != 0) {
+				thr_keydelete(res_key);
+failed:
+				mutex_unlock(&res_init_mutex);
+				return (&_res);
+			}
+			res_keys_inited = 1;
+		}
+		mutex_unlock(&res_init_mutex);
+	}
+
+	myres = thr_getspecific(res_key);
+	if (myres != NULL)
+		return (myres);
+	myres = calloc(1, sizeof(*myres));
+	if (myres == NULL)
+		return (&_res);
+#ifdef __BIND_RES_TEXT
+	myres->options = RES_TIMEOUT;	/* Motorola, et al. */
+#endif
+	if (thr_setspecific(res_key, myres) == 0)
+		return (myres);
+	free(myres);
+	return (&_res);
+}
+
+struct __res_state_ext *
+___res_ext(void)
+{
+	struct __res_state_ext *myresext;
+
+	if (res_keys_inited == 0)
+		return (&_res_ext);
+	myresext = thr_getspecific(res_ext_key);
+	if (myresext != NULL)
+		return (myresext);
+	myresext = calloc(1, sizeof(*myresext));
+	if (myresext == NULL)
+		return (&_res_ext);
+	if (thr_setspecific(res_ext_key, myresext) == 0)
+		return (myresext);
+	free(myresext);
+	return (&_res_ext);
+}
+
+struct res_send_private *
+___res_send_private(void)
+{
+	struct __res_state *myres;
+
+	myres = ___res();
+	if (sizeof(myres->pad) < sizeof(struct res_send_private))
+		abort();
+	return ((struct res_send_private *)&myres->pad);
 }
 
 /*
Index: net/res_send.c
===================================================================
RCS file: /usr/ncvs/src/lib/libc/net/res_send.c,v
retrieving revision 1.46
diff -u -u -r1.46 res_send.c
--- net/res_send.c	6 Jan 2004 18:45:13 -0000	1.46
+++ net/res_send.c	6 Feb 2004 06:07:50 -0000
@@ -101,14 +101,15 @@
 #include "un-namespace.h"
 
 #include "res_config.h"
+#include "res_send_private.h"
 
-static int s = -1;		/* socket used for communications */
-static int connected = 0;	/* is the socket connected */
-static int vc = 0;		/* is the socket a virtual circuit? */
-static int af = 0;		/* address family of socket */
-static res_send_qhook Qhook = NULL;
-static res_send_rhook Rhook = NULL;
 
+#define	s		___res_send_private()->s
+#define	connected	___res_send_private()->connected
+#define	vc		___res_send_private()->vc
+#define	af		___res_send_private()->af
+#define	Qhook		___res_send_private()->Qhook
+#define	Rhook		___res_send_private()->Rhook
 
 #define CAN_RECONNECT 1
 
@@ -123,8 +124,6 @@
 			fprintf args;\
 			__fp_nquery(query, size, stdout);\
 		} else {}
-static char abuf[NI_MAXHOST];
-static char pbuf[NI_MAXSERV];
 static void Aerror(FILE *, char *, int, struct sockaddr *);
 static void Perror(FILE *, char *, int);
 
@@ -138,6 +137,9 @@
 	int save = errno;
 
 	if (_res.options & RES_DEBUG) {
+		char abuf[NI_MAXHOST];
+		char pbuf[NI_MAXSERV];
+
 		if (getnameinfo(address, address->sa_len, abuf, sizeof(abuf),
 		    pbuf, sizeof(pbuf),
 		    NI_NUMERICHOST|NI_NUMERICSERV|NI_WITHSCOPEID) != 0) {
@@ -388,6 +390,7 @@
 	 */
 	for (try = 0; try < _res.retry; try++) {
 	    for (ns = 0; ns < _res.nscount; ns++) {
+		char abuf[NI_MAXHOST];
 		struct sockaddr *nsap = get_nsaddr(ns);
 		socklen_t salen;
 
Index: net/res_send_private.h
===================================================================
RCS file: net/res_send_private.h
diff -N net/res_send_private.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ net/res_send_private.h	6 Feb 2004 05:57:09 -0000
@@ -0,0 +1,10 @@
+struct res_send_private {
+	int s;			/* socket used for communications */
+	int connected;		/* is the socket connected */
+	int vc;			/* is the socket a virtual circuit? */
+	int af;			/* address family of socket */
+	res_send_qhook Qhook;
+	res_send_rhook Rhook;
+};
+
+struct res_send_private *___res_send_private(void);


-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green@FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200402072256.i17Muf1S099944>