This is the mail archive of the libc-alpha@sourceware.cygnus.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [artdodge@cs.bu.edu] libc/1220: The code in resolv/res_send.c is not thread-safe


>>>>> Ulrich Drepper writes:

Uli> Mark Kettenis <kettenis@wins.uva.nl> writes:
>> The resolver is indeed not thread safe.  This is a known issue, and
>> not easy to solve.

Uli> I think the existing res_send code can be made thread-safe.

Uli,  thanks for the appended patch.  Could you add it also to glibc
2.1.2, please?

Adam, could you check if this patch fixes your problem and tell me the 
result?  I'd like to know whether to close the bug report.

Thanks,
Andreas

1999-07-24  Ulrich Drepper  <drepper@cygnus.com>

        * resolv/res_send.c: Add locks for res_send and res_close use.


--- glibc-2-1-branch/resolv/res_send.c	Mon Sep 14 23:08:02 1998
+++ libc/resolv/res_send.c	Sun Jul 25 08:28:16 1999
@@ -55,7 +51,7 @@
 
 #if defined(LIBC_SCCS) && !defined(lint)
 static char sccsid[] = "@(#)res_send.c	8.1 (Berkeley) 6/4/93";
-static char rcsid[] = "$Id: res_send.c,v 1.20 1998/09/14 11:37:14 drepper Exp $";
+static char rcsid[] = "$Id: res_send.c,v 1.22 1999/07/25 00:56:12 drepper Exp $";
 #endif /* LIBC_SCCS and not lint */
 
 	/* change this to "0"
@@ -91,6 +87,13 @@
 # include "../conf/portability.h"
 #endif
 
+#include <bits/libc-lock.h>
+
+/* Lock to protect the connection use.  */
+__libc_lock_define_initialized (static, lock)
+
+static void res_close_internal (void);
+
 #if defined(USE_OPTIONS_H)
 # include <../conf/options.h>
 #endif
@@ -297,6 +300,7 @@
 	int gotsomewhere, connreset, terrno, try, v_circuit, resplen, ns;
 	register int n;
 	u_int badns;	/* XXX NSMAX can't exceed #/bits in this var */
+	int result = -1;
 
 	if ((_res.options & RES_INIT) == 0 && res_init() == -1) {
 		/* errno should have been set by res_init() in this case. */
@@ -314,6 +318,8 @@
 	terrno = ETIMEDOUT;
 	badns = 0;
 
+	__libc_lock_lock (lock);
+
 	/*
 	 * Send request, RETRY times, or until successful
 	 */
@@ -322,7 +328,7 @@
 		struct sockaddr_in *nsap = &_res.nsaddr_list[ns];
     same_ns:
 		if (badns & (1 << ns)) {
-			res_close();
+			res_close_internal();
 			goto next_ns;
 		}
 
@@ -339,10 +345,11 @@
 					done = 1;
 					break;
 				case res_nextns:
-					res_close();
+					res_close_internal();
 					goto next_ns;
 				case res_done:
-					return (resplen);
+					result = resplen;
+					goto and_out;
 				case res_modified:
 					/* give the hook another try */
 					if (++loops < 42) /*doug adams*/
@@ -351,7 +358,7 @@
 				case res_error:
 					/*FALLTHROUGH*/
 				default:
-					return (-1);
+					goto and_out;
 				}
 			} while (!done);
 		}
@@ -374,13 +381,13 @@
 			truncated = 0;
 			if ((s < 0) || (!vc)) {
 				if (s >= 0)
-					res_close();
+					res_close_internal();
 
 				s = socket(PF_INET, SOCK_STREAM, 0);
 				if (s < 0) {
 					terrno = errno;
 					Perror(stderr, "socket(vc)", errno);
-					return (-1);
+					goto and_out;
 				}
 				__set_errno (0);
 				if (connect(s, (struct sockaddr *)nsap,
@@ -389,7 +396,7 @@
 					Aerror(stderr, "connect/vc",
 					       errno, *nsap);
 					badns |= (1 << ns);
-					res_close();
+					res_close_internal();
 					goto next_ns;
 				}
 				vc = 1;
@@ -406,7 +413,7 @@
 				terrno = errno;
 				Perror(stderr, "write failed", errno);
 				badns |= (1 << ns);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			/*
@@ -423,7 +430,7 @@
 			if (n <= 0) {
 				terrno = errno;
 				Perror(stderr, "read failed", errno);
-				res_close();
+				res_close_internal();
 				/*
 				 * A long running process might get its TCP
 				 * connection reset if the remote server was
@@ -435,10 +442,10 @@
 				 */
 				if (terrno == ECONNRESET && !connreset) {
 					connreset = 1;
-					res_close();
+					res_close_internal();
 					goto same_ns;
 				}
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			resplen = _getshort(ans);
@@ -458,7 +465,7 @@
 				       (stdout, ";; undersized: %d\n", len));
 				terrno = EMSGSIZE;
 				badns |= (1 << ns);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			cp = ans;
@@ -470,7 +477,7 @@
 			if (n <= 0) {
 				terrno = errno;
 				Perror(stderr, "read(vc)", errno);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			if (truncated) {
@@ -517,7 +524,7 @@
 
 			if ((s < 0) || vc) {
 				if (vc)
-					res_close();
+					res_close_internal();
 				s = socket(PF_INET, SOCK_DGRAM, 0);
 				if (s < 0) {
 #if !CAN_RECONNECT
@@ -525,7 +532,7 @@
 #endif
 					terrno = errno;
 					Perror(stderr, "socket(dg)", errno);
-					return (-1);
+					goto and_out;
 				}
 				connected = 0;
 			}
@@ -557,7 +564,7 @@
 						       "connect(dg)",
 						       errno, *nsap);
 						badns |= (1 << ns);
-						res_close();
+						res_close_internal();
 						goto next_ns;
 					}
 					connected = 1;
@@ -565,7 +572,7 @@
 				if (send(s, (char*)buf, buflen, 0) != buflen) {
 					Perror(stderr, "send", errno);
 					badns |= (1 << ns);
-					res_close();
+					res_close_internal();
 					goto next_ns;
 				}
 			} else {
@@ -602,7 +609,7 @@
 				    != buflen) {
 					Aerror(stderr, "sendto", errno, *nsap);
 					badns |= (1 << ns);
-					res_close();
+					res_close_internal();
 					goto next_ns;
 				}
 			}
@@ -618,7 +625,7 @@
     wait:
 			if (s < 0 || s >= FD_SETSIZE) {
 				Perror(stderr, "s out-of-bounds", EMFILE);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			pfd[0].fd = s;
@@ -628,7 +635,7 @@
 				if (errno == EINTR)
 					goto wait;
 				Perror(stderr, "poll", errno);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			if (n == 0) {
@@ -638,7 +645,7 @@
 				Dprint(_res.options & RES_DEBUG,
 				       (stdout, ";; timeout\n"));
 				gotsomewhere = 1;
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			__set_errno (0);
@@ -647,7 +654,7 @@
 					   (struct sockaddr *)&from, &fromlen);
 			if (resplen <= 0) {
 				Perror(stderr, "recvfrom", errno);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			gotsomewhere = 1;
@@ -660,7 +667,7 @@
 					resplen));
 				terrno = EMSGSIZE;
 				badns |= (1 << ns);
-				res_close();
+				res_close_internal();
 				goto next_ns;
 			}
 			if (hp->id != anhp->id) {
@@ -711,7 +718,7 @@
 					(stdout, "server rejected query:\n"),
 					ans, (resplen>anssiz)?anssiz:resplen);
 				badns |= (1 << ns);
-				res_close();
+				res_close_internal();
 				/* don't retry if called from dig */
 				if (!_res.pfcode)
 					goto next_ns;
@@ -724,7 +731,7 @@
 				Dprint(_res.options & RES_DEBUG,
 				       (stdout, ";; truncated answer\n"));
 				v_circuit = 1;
-				res_close();
+				res_close_internal();
 				goto same_ns;
 			}
 		} /*if vc/dg*/
@@ -746,7 +753,7 @@
 		 */
 		if ((v_circuit && (!(_res.options & RES_USEVC) || ns != 0)) ||
 		    !(_res.options & RES_STAYOPEN)) {
-			res_close();
+			res_close_internal();
 		}
 		if (Rhook) {
 			int done = 0, loops = 0;
@@ -762,7 +769,7 @@
 					done = 1;
 					break;
 				case res_nextns:
-					res_close();
+					res_close_internal();
 					goto next_ns;
 				case res_modified:
 					/* give the hook another try */
@@ -772,16 +779,17 @@
 				case res_error:
 					/*FALLTHROUGH*/
 				default:
-					return (-1);
+					goto and_out;
 				}
 			} while (!done);
 
 		}
-		return (resplen);
+		result = resplen;
+		goto and_out;
     next_ns: ;
 	   } /*foreach ns*/
 	} /*foreach retry*/
-	res_close();
+	res_close_internal();
 	if (!v_circuit) {
 		if (!gotsomewhere)
 			__set_errno (ECONNREFUSED); /* no nameservers found */
@@ -789,7 +797,11 @@
 			__set_errno (ETIMEDOUT);    /* no answer obtained */
 	} else
 		__set_errno (terrno);
-	return (-1);
+
+ and_out:
+	__libc_lock_unlock (lock);
+
+	return result;
 }
 
 /*
@@ -799,8 +811,8 @@
  *
  * This routine is not expected to be user visible.
  */
-void
-res_close()
+static void
+res_close_internal()
 {
 	if (s >= 0) {
 		(void) close(s);
@@ -810,6 +822,14 @@
 	}
 }
 
+void
+res_close ()
+{
+	__libc_lock_lock (lock);
+	res_close_internal ();
+	__libc_lock_unlock (lock);
+}
+
 #ifdef ultrix
 /* ultrix 4.0 had some icky packaging in its libc.a.  alias for it here.
  * there is more gunk of this kind over in res_debug.c.

-- 
 Andreas Jaeger   aj@arthur.rhein-neckar.de    jaeger@informatik.uni-kl.de
  for pgp-key finger ajaeger@aixd1.rhrk.uni-kl.de

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]