From 08a2d2d37aa4f6bfb76d04b09e6ffaedeb90154d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Witold=20Kr=C4=99cicki?= Date: Thu, 4 Oct 2018 14:52:54 +0000 Subject: [PATCH] Make watching/unwatching sockets saner. --- lib/isc/unix/socket.c | 93 +++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 51 deletions(-) diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c index f963b0cf66..1b148569a2 100644 --- a/lib/isc/unix/socket.c +++ b/lib/isc/unix/socket.c @@ -2893,6 +2893,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; } @@ -3018,9 +3019,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); @@ -3124,7 +3125,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; } @@ -3139,7 +3140,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: /* @@ -3152,7 +3153,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: @@ -3163,11 +3164,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); } @@ -3180,8 +3180,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", @@ -3194,7 +3193,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: @@ -3205,10 +3204,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); } @@ -3221,7 +3220,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); /* @@ -3238,9 +3236,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; @@ -3253,7 +3250,6 @@ process_fd(isc__socketthread_t *thread, int fd, bool readable, internal_accept(sock); else internal_recv(sock); - unwatch_read = true; } if (writeable) { @@ -3261,15 +3257,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); @@ -4129,10 +4120,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", @@ -4277,12 +4270,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); @@ -4843,15 +4837,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); } @@ -5004,13 +4995,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); @@ -5039,8 +5030,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); @@ -5063,10 +5053,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; } @@ -5116,6 +5103,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); } @@ -5537,7 +5528,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", @@ -5639,7 +5630,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);