From: Witold Kręcicki <wpk@isc.org>
Date: Thu, 4 Oct 2018 14:52:54 +0000 (+0000)
Subject: Make watching/unwatching sockets saner.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9e4e71b57e9782576ea76b78b7741e53f50924e3;p=thirdparty%2Fbind9.git

Make watching/unwatching sockets saner.
---

diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c
index 2fb55d49e6e..38a057b708d 100644
--- a/lib/isc/unix/socket.c
+++ b/lib/isc/unix/socket.c
@@ -2894,6 +2894,7 @@ internal_accept(isc__socket_t *sock) {
 	 */
 	dev = ISC_LIST_HEAD(sock->accept_list);
 	if (dev == NULL) {
+		unwatch_fd(thread, sock->fd, SELECT_POKE_ACCEPT);
 		UNLOCK(&sock->lock);
 		return;
 	}
@@ -3019,9 +3020,9 @@ internal_accept(isc__socket_t *sock) {
 	/*
 	 * Poke watcher if there are more pending accepts.
 	 */
-	if (!ISC_LIST_EMPTY(sock->accept_list))
-		watch_fd(thread, sock->fd,
-			 SELECT_POKE_ACCEPT);
+	if (ISC_LIST_EMPTY(sock->accept_list))
+		unwatch_fd(thread, sock->fd,
+			   SELECT_POKE_ACCEPT);
 
 	UNLOCK(&sock->lock);
 
@@ -3125,7 +3126,7 @@ internal_recv(isc__socket_t *sock) {
 	LOCK(&sock->lock);
 	dev = ISC_LIST_HEAD(sock->recv_list);
 	if (dev == NULL) {
-		UNLOCK(&sock->lock);
+		goto finish;
 		return;
 	}
 
@@ -3140,7 +3141,7 @@ internal_recv(isc__socket_t *sock) {
 	while (dev != NULL) {
 		switch (doio_recv(sock, dev)) {
 		case DOIO_SOFT:
-			goto poke;
+			goto finish;
 
 		case DOIO_EOF:
 			/*
@@ -3153,7 +3154,7 @@ internal_recv(isc__socket_t *sock) {
 				send_recvdone_event(sock, &dev);
 				dev = ISC_LIST_HEAD(sock->recv_list);
 			} while (dev != NULL);
-			goto poke;
+			goto finish;
 
 		case DOIO_SUCCESS:
 		case DOIO_HARD:
@@ -3164,11 +3165,10 @@ internal_recv(isc__socket_t *sock) {
 		dev = ISC_LIST_HEAD(sock->recv_list);
 	}
 
- poke:
-	if (!ISC_LIST_EMPTY(sock->recv_list))
-		watch_fd(&sock->manager->threads[sock->threadid], sock->fd,
-			 SELECT_POKE_READ);
-
+ finish:
+	if (ISC_LIST_EMPTY(sock->recv_list))
+		unwatch_fd(&sock->manager->threads[sock->threadid], sock->fd,
+			   SELECT_POKE_READ);
 	UNLOCK(&sock->lock);
 }
 
@@ -3181,8 +3181,7 @@ internal_send(isc__socket_t *sock) {
 	LOCK(&sock->lock);
 	dev = ISC_LIST_HEAD(sock->send_list);
 	if (dev == NULL) {
-		UNLOCK(&sock->lock);
-		return;
+		goto finish;
 	}
 	socket_log(sock, NULL, EVENT, NULL, 0, 0,
 		   "internal_send:  event %p -> task %p",
@@ -3195,7 +3194,7 @@ internal_send(isc__socket_t *sock) {
 	while (dev != NULL) {
 		switch (doio_send(sock, dev)) {
 		case DOIO_SOFT:
-			goto poke;
+			goto finish;
 
 		case DOIO_HARD:
 		case DOIO_SUCCESS:
@@ -3206,10 +3205,10 @@ internal_send(isc__socket_t *sock) {
 		dev = ISC_LIST_HEAD(sock->send_list);
 	}
 
- poke:
-	if (!ISC_LIST_EMPTY(sock->send_list))
-		watch_fd(&sock->manager->threads[sock->threadid], sock->fd, SELECT_POKE_WRITE);
-
+ finish:
+	if (ISC_LIST_EMPTY(sock->send_list))
+		unwatch_fd(&sock->manager->threads[sock->threadid],
+			   sock->fd, SELECT_POKE_WRITE);
 	UNLOCK(&sock->lock);
 }
 
@@ -3222,7 +3221,6 @@ process_fd(isc__socketthread_t *thread, int fd, bool readable,
 	   bool writeable)
 {
 	isc__socket_t *sock;
-	bool unwatch_read = false, unwatch_write = false;
 	int lockid = FDLOCK_ID(fd);
 
 	/*
@@ -3239,9 +3237,8 @@ process_fd(isc__socketthread_t *thread, int fd, bool readable,
 
 	sock = thread->fds[fd];
 	if (sock == NULL) {
-		unwatch_read = readable;
-		unwatch_write = writeable;
-		goto unlock_fd;
+		UNLOCK(&thread->fdlock[lockid]);
+		return;
 	}
 	if (SOCK_DEAD(sock)) { /* Sock is being closed, bail */
 		goto unlock_fd;
@@ -3254,7 +3251,6 @@ process_fd(isc__socketthread_t *thread, int fd, bool readable,
 			internal_accept(sock);
 		else
 			internal_recv(sock);
-		unwatch_read = true;
 	}
 
 	if (writeable) {
@@ -3262,15 +3258,10 @@ process_fd(isc__socketthread_t *thread, int fd, bool readable,
 			internal_connect(sock);
 		else
 			internal_send(sock);
-		unwatch_write = true;
 	}
 
  unlock_fd:
 	UNLOCK(&thread->fdlock[lockid]);
-	if (unwatch_read)
-		(void)unwatch_fd(thread, fd, SELECT_POKE_READ);
-	if (unwatch_write)
-		(void)unwatch_fd(thread, fd, SELECT_POKE_WRITE);
 	if (sock != NULL) {
 		if (isc_refcount_decrement(&sock->references) == 1) {
 			destroy(&sock);
@@ -4130,10 +4121,12 @@ socket_recv(isc__socket_t *sock, isc_socketevent_t *dev, isc_task_t *task,
 		 * Enqueue the request.  If the socket was previously not being
 		 * watched, poke the watcher to start paying attention to it.
 		 */
-		if (ISC_LIST_EMPTY(sock->recv_list))
+		bool do_poke = ISC_LIST_EMPTY(sock->recv_list);
+		ISC_LIST_ENQUEUE(sock->recv_list, dev, ev_link);
+		if (do_poke) {
 			select_poke(sock->manager, sock->threadid, sock->fd,
 				    SELECT_POKE_READ);
-		ISC_LIST_ENQUEUE(sock->recv_list, dev, ev_link);
+		}
 
 		socket_log(sock, NULL, EVENT, NULL, 0, 0,
 			   "socket_recv: event %p -> task %p",
@@ -4278,12 +4271,13 @@ socket_send(isc__socket_t *sock, isc_socketevent_t *dev, isc_task_t *task,
 			 * not being watched, poke the watcher to start
 			 * paying attention to it.
 			 */
-			if (ISC_LIST_EMPTY(sock->send_list))
+			bool do_poke = ISC_LIST_EMPTY(sock->send_list);
+			ISC_LIST_ENQUEUE(sock->send_list, dev, ev_link);
+			if (do_poke) {
 				select_poke(sock->manager, sock->threadid,
 					    sock->fd,
 					    SELECT_POKE_WRITE);
-			ISC_LIST_ENQUEUE(sock->send_list, dev, ev_link);
-
+			}
 			socket_log(sock, NULL, EVENT, NULL, 0, 0,
 				   "socket_send: event %p -> task %p",
 				   dev, ntask);
@@ -4844,15 +4838,12 @@ isc_socket_accept(isc_socket_t *sock0,
 	 * is no race condition.  We will keep the lock for such a short
 	 * bit of time waking it up now or later won't matter all that much.
 	 */
-	if (ISC_LIST_EMPTY(sock->accept_list))
-		do_poke = true;
-
+	do_poke = ISC_LIST_EMPTY(sock->accept_list);
 	ISC_LIST_ENQUEUE(sock->accept_list, dev, ev_link);
-
-	if (do_poke)
+	if (do_poke) {
 		select_poke(manager, sock->threadid, sock->fd,
 			    SELECT_POKE_ACCEPT);
-
+	}
 	UNLOCK(&sock->lock);
 	return (ISC_R_SUCCESS);
 }
@@ -5005,13 +4996,13 @@ isc_socket_connect(isc_socket_t *sock0, const isc_sockaddr_t *addr,
 	 * is no race condition.  We will keep the lock for such a short
 	 * bit of time waking it up now or later won't matter all that much.
 	 */
-	if (ISC_LIST_EMPTY(sock->connect_list) && !sock->connecting)
+	bool do_poke = ISC_LIST_EMPTY(sock->connect_list);
+	ISC_LIST_ENQUEUE(sock->connect_list, dev, ev_link);
+	if (do_poke && !sock->connecting) {
+		sock->connecting = 1;
 		select_poke(manager, sock->threadid, sock->fd,
 			    SELECT_POKE_CONNECT);
-
-	sock->connecting = 1;
-
-	ISC_LIST_ENQUEUE(sock->connect_list, dev, ev_link);
+	}
 
 	UNLOCK(&sock->lock);
 	return (ISC_R_SUCCESS);
@@ -5040,8 +5031,7 @@ internal_connect(isc__socket_t *sock) {
 	dev = ISC_LIST_HEAD(sock->connect_list);
 	if (dev == NULL) {
 		INSIST(!sock->connecting);
-		UNLOCK(&sock->lock);
-		return;
+		goto finish;
 	}
 
 	INSIST(sock->connecting);
@@ -5064,10 +5054,7 @@ internal_connect(isc__socket_t *sock) {
 		 */
 		if (SOFT_ERROR(errno) || errno == EINPROGRESS) {
 			sock->connecting = 1;
-			watch_fd(&sock->manager->threads[sock->threadid], sock->fd,
-				 SELECT_POKE_CONNECT);
 			UNLOCK(&sock->lock);
-
 			return;
 		}
 
@@ -5117,6 +5104,10 @@ internal_connect(isc__socket_t *sock) {
 		dev = ISC_LIST_HEAD(sock->connect_list);
 	} while (dev != NULL);
 
+ finish:
+	unwatch_fd(&sock->manager->threads[sock->threadid], sock->fd,
+		   SELECT_POKE_CONNECT);
+
 	UNLOCK(&sock->lock);
 }
 
@@ -5538,7 +5529,7 @@ isc_socketmgr_renderxml(isc_socketmgr_t *mgr0, xmlTextWriterPtr writer) {
 		TRY0(xmlTextWriterStartElement(writer,
 					       ISC_XMLCHAR "references"));
 		TRY0(xmlTextWriterWriteFormatString(writer, "%d",
-				    isc_refcount_current(&sock->references)));
+				    (int)isc_refcount_current(&sock->references)));
 		TRY0(xmlTextWriterEndElement(writer));
 
 		TRY0(xmlTextWriterWriteElement(writer, ISC_XMLCHAR "type",
@@ -5640,7 +5631,7 @@ isc_socketmgr_renderjson(isc_socketmgr_t *mgr0, json_object *stats) {
 			json_object_object_add(entry, "name", obj);
 		}
 
-		obj = json_object_new_int(isc_refcount_current(&sock->references));
+		obj = json_object_new_int((int)isc_refcount_current(&sock->references));
 		CHECKMEM(obj);
 		json_object_object_add(entry, "references", obj);