4537. [bug] Handle timouts better in dig/host/nslookup. [RT #43576]

CHANGES

@@ -1,3 +1,5 @@
+4537.	[bug]		Handle timouts better in dig/host/nslookup. [RT #43576]
+
 4536.	[bug]		ISC_SOCKEVENTATTR_USEMINMTU was not being cleared
 			when reusing the event structure. [RT #43885]
 

bin/dig/dighost.c

@@ -759,7 +759,6 @@ make_empty_lookup(void) {
 	looknew->sendmsg = NULL;
 	looknew->name = NULL;
 	looknew->oname = NULL;
-	looknew->timer = NULL;
 	looknew->xfr_q = NULL;
 	looknew->current_query = NULL;
 	looknew->doing_xfr = ISC_FALSE;
@@ -1691,6 +1690,8 @@ clear_query(dig_query_t *query) {
 
 	debug("clear_query(%p)", query);
 
+	if (query->timer != NULL)
+		isc_timer_detach(&query->timer);
 	lookup = query->lookup;
 
 	if (lookup->current_query == query)
@@ -1784,8 +1785,6 @@ destroy_lookup(dig_lookup_t *lookup) {
 		debug("freeing buffer %p", lookup->querysig);
 		isc_buffer_free(&lookup->querysig);
 	}
-	if (lookup->timer != NULL)
-		isc_timer_detach(&lookup->timer);
 	if (lookup->sendspace != NULL)
 		isc_mempool_put(commctx, lookup->sendspace);
 
@@ -2692,6 +2691,7 @@ setup_lookup(dig_lookup_t *lookup) {
 		debug("create query %p linked to lookup %p",
 		       query, lookup);
 		query->lookup = lookup;
+		query->timer = NULL;
 		query->waiting_connect = ISC_FALSE;
 		query->waiting_senddone = ISC_FALSE;
 		query->pending_free = ISC_FALSE;
@@ -2701,6 +2701,7 @@ setup_lookup(dig_lookup_t *lookup) {
 		query->second_rr_rcvd = ISC_FALSE;
 		query->first_repeat_rcvd = ISC_FALSE;
 		query->warn_id = ISC_TRUE;
+		query->timedout = ISC_FALSE;
 		query->first_rr_serial = 0;
 		query->second_rr_serial = 0;
 		query->servname = serv->servername;
@@ -2809,8 +2810,6 @@ cancel_lookup(dig_lookup_t *lookup) {
 		}
 		query = next;
 	}
-	if (lookup->timer != NULL)
-		isc_timer_detach(&lookup->timer);
 	lookup->pending = ISC_FALSE;
 	lookup->retries = 0;
 }
@@ -2828,7 +2827,7 @@ bringup_timer(dig_query_t *query, unsigned int default_timeout) {
 	 * just reset it.
 	 */
 	l = query->lookup;
-	if (ISC_LIST_NEXT(query, link) != NULL)
+	if (ISC_LINK_LINKED(query, link) && ISC_LIST_NEXT(query, link) != NULL)
 		local_timeout = SERVER_TIMEOUT;
 	else {
 		if (timeout == 0)
@@ -2838,21 +2837,21 @@ bringup_timer(dig_query_t *query, unsigned int default_timeout) {
 	}
 	debug("have local timeout of %d", local_timeout);
 	isc_interval_set(&l->interval, local_timeout, 0);
-	if (l->timer != NULL)
-		isc_timer_detach(&l->timer);
+	if (query->timer != NULL)
+		isc_timer_detach(&query->timer);
 	result = isc_timer_create(timermgr, isc_timertype_once, NULL,
 				  &l->interval, global_task, connect_timeout,
-				  l, &l->timer);
+				  query, &query->timer);
 	check_result(result, "isc_timer_create");
 }
 
 static void
-force_timeout(dig_lookup_t *l, dig_query_t *query) {
+force_timeout(dig_query_t *query) {
 	isc_event_t *event;
 
 	debug("force_timeout ()");
 	event = isc_event_allocate(mctx, query, ISC_TIMEREVENT_IDLE,
-				   connect_timeout, l,
+				   connect_timeout, query,
 				   sizeof(isc_event_t));
 	if (event == NULL) {
 		fatal("isc_event_allocate: %s",
@@ -2866,8 +2865,8 @@ force_timeout(dig_lookup_t *l, dig_query_t *query) {
 	 * We need to cancel the possible timeout event not to confuse
 	 * ourselves due to the duplicate events.
 	 */
-	if (l->timer != NULL)
-		isc_timer_detach(&l->timer);
+	if (query->timer != NULL)
+		isc_timer_detach(&query->timer);
 }
 
 
@@ -2897,7 +2896,7 @@ send_tcp_connect(dig_query_t *query) {
 		 * by triggering an immediate 'timeout' (we lie, but the effect
 		 * is the same).
 		 */
-		force_timeout(l, query);
+		force_timeout(query);
 		return;
 	}
 
@@ -2911,7 +2910,10 @@ send_tcp_connect(dig_query_t *query) {
 		printf(";; Skipping mapped address '%s'\n", buf);
 
 		query->waiting_connect = ISC_FALSE;
-		next = ISC_LIST_NEXT(query, link);
+		if (ISC_LINK_LINKED(query, link))
+			next = ISC_LIST_NEXT(query, link);
+		else
+			next = NULL;
 		l = query->lookup;
 		clear_query(query);
 		if (next == NULL) {
@@ -2929,7 +2931,10 @@ send_tcp_connect(dig_query_t *query) {
 		printf(";; Skipping server %s, incompatible "
 		       "address family\n", query->servname);
 		query->waiting_connect = ISC_FALSE;
-		next = ISC_LIST_NEXT(query, link);
+		if (ISC_LINK_LINKED(query, link))
+			next = ISC_LIST_NEXT(query, link);
+		else
+			next = NULL;
 		l = query->lookup;
 		clear_query(query);
 		if (next == NULL) {
@@ -2983,9 +2988,11 @@ send_tcp_connect(dig_query_t *query) {
 	 */
 	if (l->ns_search_only && !l->trace_root) {
 		debug("sending next, since searching");
-		next = ISC_LIST_NEXT(query, link);
-		if (ISC_LINK_LINKED(query, link))
+		if (ISC_LINK_LINKED(query, link)) {
+			next = ISC_LIST_NEXT(query, link);
 			ISC_LIST_DEQUEUE(l->q, query, link);
+		} else
+			next = NULL;
 		ISC_LIST_ENQUEUE(l->connecting, query, clink);
 		if (next != NULL)
 			send_tcp_connect(next);
@@ -3027,7 +3034,7 @@ send_udp(dig_query_t *query) {
 		result = get_address(query->servname, port, &query->sockaddr);
 		if (result != ISC_R_SUCCESS) {
 			/* This servname doesn't have an address. */
-			force_timeout(l, query);
+			force_timeout(query);
 			return;
 		}
 
@@ -3105,7 +3112,7 @@ send_udp(dig_query_t *query) {
 static void
 connect_timeout(isc_task_t *task, isc_event_t *event) {
 	dig_lookup_t *l = NULL;
-	dig_query_t *query = NULL, *next, *cq;
+	dig_query_t *query = NULL, *cq;
 
 	UNUSED(task);
 	REQUIRE(event->ev_type == ISC_TIMEREVENT_IDLE);
@@ -3113,13 +3120,14 @@ connect_timeout(isc_task_t *task, isc_event_t *event) {
 	debug("connect_timeout()");
 
 	LOCK_LOOKUP;
-	l = event->ev_arg;
-	query = l->current_query;
+	query = event->ev_arg;
+	l = query->lookup;
 	isc_event_free(&event);
 
 	INSIST(!free_now);
 
 	if ((query != NULL) && (query->lookup->current_query != NULL) &&
+	    ISC_LINK_LINKED(query->lookup->current_query, link) &&
 	    (ISC_LIST_NEXT(query->lookup->current_query, link) != NULL)) {
 		debug("trying next server...");
 		cq = query->lookup->current_query;
@@ -3129,14 +3137,17 @@ connect_timeout(isc_task_t *task, isc_event_t *event) {
 			if (query->sock != NULL)
 				isc_socket_cancel(query->sock, NULL,
 						  ISC_SOCKCANCEL_ALL);
-			next = ISC_LIST_NEXT(cq, link);
-			if (next != NULL)
-				send_tcp_connect(next);
+			send_tcp_connect(ISC_LIST_NEXT(cq, link));
 		}
 		UNLOCK_LOOKUP;
 		return;
 	}
 
+	if (l->tcp_mode && query->sock != NULL) {
+		query->timedout = ISC_TRUE;
+		isc_socket_cancel(query->sock, NULL, ISC_SOCKCANCEL_ALL);
+	}
+
 	if (l->retries > 1) {
 		if (!l->tcp_mode) {
 			l->retries--;
@@ -3151,9 +3162,11 @@ connect_timeout(isc_task_t *task, isc_event_t *event) {
 			check_next_lookup(l);
 		}
 	} else {
-		fputs(l->cmdline, stdout);
-		printf(";; connection timed out; no servers could be "
-		       "reached\n");
+		if (!l->ns_search_only) {
+			fputs(l->cmdline, stdout);
+			printf(";; connection timed out; no servers could be "
+			       "reached\n");
+		}
 		cancel_lookup(l);
 		check_next_lookup(l);
 		if (exitcode < 9)
@@ -3319,6 +3332,7 @@ launch_next_query(dig_query_t *query, isc_boolean_t include_question) {
  */
 static void
 connect_done(isc_task_t *task, isc_event_t *event) {
+	char sockstr[ISC_SOCKADDR_FORMATSIZE];
 	isc_socketevent_t *sevent = NULL;
 	dig_query_t *query = NULL, *next;
 	dig_lookup_t *l;
@@ -3340,6 +3354,12 @@ connect_done(isc_task_t *task, isc_event_t *event) {
 
 	if (sevent->result == ISC_R_CANCELED) {
 		debug("in cancel handler");
+		isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr));
+		if (query->timedout)
+			printf(";; Connection to %s(%s) for %s failed: %s.\n",
+			       sockstr, query->servname,
+			       query->lookup->textname,
+			       isc_result_totext(ISC_R_TIMEDOUT));
 		isc_socket_detach(&query->sock);
 		INSIST(sockcount > 0);
 		sockcount--;
@@ -3353,7 +3373,6 @@ connect_done(isc_task_t *task, isc_event_t *event) {
 		return;
 	}
 	if (sevent->result != ISC_R_SUCCESS) {
-		char sockstr[ISC_SOCKADDR_FORMATSIZE];
 
 		debug("unsuccessful connection: %s",
 		      isc_result_totext(sevent->result));
@@ -3364,8 +3383,8 @@ connect_done(isc_task_t *task, isc_event_t *event) {
 			       query->servname, query->lookup->textname,
 			       isc_result_totext(sevent->result));
 		isc_socket_detach(&query->sock);
+		INSIST(sockcount > 0);
 		sockcount--;
-		INSIST(sockcount >= 0);
 		/* XXX Clean up exitcodes */
 		if (exitcode < 9)
 			exitcode = 9;
@@ -3690,8 +3709,8 @@ recv_done(isc_task_t *task, isc_event_t *event) {
 	INSIST(b == &query->recvbuf);
 	ISC_LIST_DEQUEUE(sevent->bufferlist, &query->recvbuf, link);
 
-	if ((l->tcp_mode) && (l->timer != NULL))
-		isc_timer_touch(l->timer);
+	if ((l->tcp_mode) && (query->timer != NULL))
+		isc_timer_touch(query->timer);
 	if ((!l->pending && !l->ns_search_only) || cancel_now) {
 		debug("no longer pending.  Got %s",
 			isc_result_totext(sevent->result));
@@ -4023,7 +4042,7 @@ recv_done(isc_task_t *task, isc_event_t *event) {
 		 * the timeout to much longer, so brief network
 		 * outages won't cause the XFR to abort
 		 */
-		if (timeout != INT_MAX && l->timer != NULL) {
+		if (timeout != INT_MAX && query->timer != NULL) {
 			unsigned int local_timeout;
 
 			if (timeout == 0) {
@@ -4039,7 +4058,7 @@ recv_done(isc_task_t *task, isc_event_t *event) {
 			}
 			debug("have local timeout of %d", local_timeout);
 			isc_interval_set(&l->interval, local_timeout, 0);
-			result = isc_timer_reset(l->timer,
+			result = isc_timer_reset(query->timer,
 						 isc_timertype_once,
 						 NULL,
 						 &l->interval,
@@ -4329,8 +4348,6 @@ cancel_all(void) {
 	}
 	cancel_now = ISC_TRUE;
 	if (current_lookup != NULL) {
-		if (current_lookup->timer != NULL)
-			isc_timer_detach(&current_lookup->timer);
 		for (q = ISC_LIST_HEAD(current_lookup->q);
 		     q != NULL;
 		     q = nq)

bin/dig/include/dig/dig.h

@@ -170,7 +170,6 @@ isc_boolean_t	sigchase;
 	isc_buffer_t renderbuf;
 	char *sendspace;
 	dns_name_t *name;
-	isc_timer_t *timer;
 	isc_interval_t interval;
 	dns_message_t *sendmsg;
 	dns_name_t *oname;
@@ -213,7 +212,8 @@ struct dig_query {
 		second_rr_rcvd,
 		first_repeat_rcvd,
 		recv_made,
-		warn_id;
+		warn_id,
+		timedout;
 	isc_uint32_t first_rr_serial;
 	isc_uint32_t second_rr_serial;
 	isc_uint32_t msg_count;
@@ -238,6 +238,7 @@ struct dig_query {
 	isc_time_t time_recv;
 	isc_uint64_t byte_count;
 	isc_buffer_t sendbuf;
+	isc_timer_t *timer;
 };
 
 struct dig_server {

bin/dig/nslookup.c

@@ -18,7 +18,6 @@
 #include <isc/parseint.h>
 #include <isc/print.h>
 #include <isc/string.h>
-#include <isc/timer.h>
 #include <isc/util.h>
 #include <isc/task.h>
 #include <isc/netaddr.h>
@@ -947,8 +946,6 @@ flush_lookup_list(void) {
 		}
 		if (l->sendmsg != NULL)
 			dns_message_destroy(&l->sendmsg);
-		if (l->timer != NULL)
-			isc_timer_detach(&l->timer);
 		lp = l;
 		l = ISC_LIST_NEXT(l, link);
 		ISC_LIST_DEQUEUE(lookup_list, lp, link);