Attach the accept "client" socket to .listener member of the socket

When accepting a TCP connection in the higher layers (tlsstream,
streamdns, and http) attach to the socket the connection was accepted
on, and use this socket instead of the parent listening socket.

This has an advantage - accessing the sock->listener now doesn't break
the thread boundaries, so we can properly check whether the socket is
being closed without requiring .closing member to be atomic_bool.

lib/isc/netmgr/http.c

@@ -1688,11 +1688,9 @@ find_server_request_handler(const char *request_path,
 
 	REQUIRE(VALID_NMSOCK(serversocket));
 
-	if (serversocket->listening) {
 	handler = http_endpoints_find(
-			request_path,
-			http_get_listener_endpoints(serversocket, tid));
-	}
+		request_path, http_get_listener_endpoints(serversocket, tid));
+
 	return (handler);
 }
 
@@ -2430,57 +2428,31 @@ http_transpost_tcp_nodelay(isc_nmhandle_t *transphandle) {
 
 static isc_result_t
 httplisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
-	isc_nmsocket_t *httplistensock = (isc_nmsocket_t *)cbarg;
+	isc_nmsocket_t *httpserver = (isc_nmsocket_t *)cbarg;
 	isc_nm_http_session_t *session = NULL;
-	isc_nmsocket_t *listener = NULL, *httpserver = NULL;
 
 	REQUIRE(VALID_NMHANDLE(handle));
 	REQUIRE(VALID_NMSOCK(handle->sock));
 
-	if (handle->sock->type == isc_nm_tlssocket) {
-		REQUIRE(VALID_NMSOCK(handle->sock->listener));
-		listener = handle->sock->listener;
-		httpserver = listener->h2.httpserver;
-	} else {
-		REQUIRE(VALID_NMSOCK(handle->sock->server));
-		listener = handle->sock->server;
-		REQUIRE(VALID_NMSOCK(listener->parent));
-		httpserver = listener->parent->h2.httpserver;
-	}
-
-	/*
-	 * NOTE: HTTP listener socket might be destroyed by the time this
-	 * function gets invoked, so we need to do extra sanity checks to
-	 * detect this case.
-	 */
 	if (isc__nm_closing(handle->sock->worker)) {
 		return (ISC_R_SHUTTINGDOWN);
-	} else if (isc__nmsocket_closing(handle->sock) || httpserver == NULL) {
-		return (ISC_R_CANCELED);
-	}
-
-	if (result != ISC_R_SUCCESS) {
-		/* XXXWPK do nothing? */
+	} else if (result != ISC_R_SUCCESS) {
 		return (result);
 	}
 
-	REQUIRE(VALID_NMSOCK(httplistensock));
-	INSIST(httplistensock == httpserver);
-
-	if (httplistensock->closing) {
-		return (ISC_R_CANCELED);
-	}
+	REQUIRE(VALID_NMSOCK(httpserver));
+	REQUIRE(httpserver->type == isc_nm_httplistener);
 
 	http_transpost_tcp_nodelay(handle);
 
 	new_session(handle->sock->worker->mctx, NULL, &session);
 	session->max_concurrent_streams =
-		atomic_load_relaxed(&httplistensock->h2.max_concurrent_streams);
+		atomic_load_relaxed(&httpserver->h2.max_concurrent_streams);
 	initialize_nghttp2_server_session(session);
 	handle->sock->h2.session = session;
 
 	isc_nmhandle_attach(handle, &session->handle);
-	isc__nmsocket_attach(httplistensock, &session->serversocket);
+	isc__nmsocket_attach(httpserver, &session->serversocket);
 	server_send_connection_header(session);
 
 	/* TODO H2 */
@@ -2528,14 +2500,9 @@ isc_nm_listenhttp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
 		return (result);
 	}
 
-	isc__nmsocket_attach(sock, &sock->outer->h2.httpserver);
-
 	sock->nchildren = sock->outer->nchildren;
 	sock->fd = (uv_os_sock_t)-1;
 
-	isc__nmsocket_barrier_init(sock);
-
-	sock->listening = true;
 	*sockp = sock;
 	return (ISC_R_SUCCESS);
 }
@@ -3204,13 +3171,6 @@ isc__nm_http_initsocket(isc_nmsocket_t *sock) {
 
 void
 isc__nm_http_cleanup_data(isc_nmsocket_t *sock) {
-	if ((sock->type == isc_nm_tcplistener ||
-	     sock->type == isc_nm_tlslistener) &&
-	    sock->h2.httpserver != NULL)
-	{
-		isc__nmsocket_detach(&sock->h2.httpserver);
-	}
-
 	if (sock->type == isc_nm_httplistener ||
 	    sock->type == isc_nm_httpsocket)
 	{

lib/isc/netmgr/netmgr-int.h

@@ -430,8 +430,6 @@ typedef struct isc_nmsocket_h2 {
 	int32_t stream_id;
 	isc_nm_http_session_t *session;
 
-	isc_nmsocket_t *httpserver;
-
 	/* maximum concurrent streams (server-side) */
 	atomic_uint_fast32_t max_concurrent_streams;
 
@@ -486,8 +484,6 @@ struct isc_nmsocket {
 
 	/*% Parent socket for multithreaded listeners */
 	isc_nmsocket_t *parent;
-	/*% Listener socket this connection was accepted on */
-	isc_nmsocket_t *listener;
 
 	/*% TLS stuff */
 	struct tlsstream {
@@ -562,6 +558,9 @@ struct isc_nmsocket {
 	/*% server socket for connections */
 	isc_nmsocket_t *server;
 
+	/*% client socket for connections */
+	isc_nmsocket_t *listener;
+
 	/*% Child sockets for multi-socket setups */
 	isc_nmsocket_t *children;
 	uint_fast32_t nchildren;
@@ -597,7 +596,6 @@ struct isc_nmsocket {
 	 */
 	bool closing;
 	bool closed;
-	bool listening;
 	bool connecting;
 	bool connected;
 	bool accepting;

lib/isc/netmgr/netmgr.c

@@ -942,7 +942,7 @@ nmhandle_free(isc_nmsocket_t *sock, isc_nmhandle_t *handle) {
 		handle->dofree(handle->opaque);
 	}
 
-	isc_mem_put(sock->worker->mctx, handle, sizeof(isc_nmhandle_t));
+	isc_mem_put(sock->worker->mctx, handle, sizeof(*handle));
 }
 
 static void
@@ -1438,7 +1438,7 @@ isc__nm_closing(isc__networker_t *worker) {
 
 bool
 isc__nmsocket_closing(isc_nmsocket_t *sock) {
-	return (!isc__nmsocket_active(sock) || sock->closing ||
+	return (!sock->active || sock->closing ||
 		isc__nm_closing(sock->worker) ||
 		(sock->server != NULL && !isc__nmsocket_active(sock->server)));
 }
@@ -1746,42 +1746,27 @@ isc_nm_stoplistening(isc_nmsocket_t *sock) {
 	}
 }
 
-static void
-nmsocket_stop_cb(void *arg) {
-	isc_nmsocket_t *listener = arg;
-
-	isc_barrier_wait(&listener->stop_barrier);
-}
-
 void
 isc__nmsocket_stop(isc_nmsocket_t *listener) {
 	REQUIRE(VALID_NMSOCK(listener));
 	REQUIRE(listener->tid == isc_tid());
 	REQUIRE(listener->tid == 0);
-	REQUIRE(listener->listening);
+	REQUIRE(listener->type == isc_nm_httplistener ||
+		listener->type == isc_nm_tlslistener ||
+		listener->type == isc_nm_streamdnslistener);
 	REQUIRE(!listener->closing);
 
 	listener->closing = true;
 
-	for (size_t i = 1; i < listener->nchildren; i++) {
-		isc__networker_t *worker =
-			&listener->worker->netmgr->workers[i];
-		isc_async_run(worker->loop, nmsocket_stop_cb, listener);
-	}
-
-	nmsocket_stop_cb(listener);
-
-	listener->listening = false;
+	REQUIRE(listener->outer != NULL);
+	isc_nm_stoplistening(listener->outer);
 
 	listener->accept_cb = NULL;
 	listener->accept_cbarg = NULL;
 	listener->recv_cb = NULL;
 	listener->recv_cbarg = NULL;
 
-	if (listener->outer != NULL) {
-		isc_nm_stoplistening(listener->outer);
 	isc__nmsocket_detach(&listener->outer);
-	}
 
 	listener->closed = true;
 }

lib/isc/netmgr/streamdns.c

@@ -642,25 +642,21 @@ streamdns_accept_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	isc_nmsocket_t *listensock = (isc_nmsocket_t *)cbarg;
 	isc_nmsocket_t *nsock;
 	isc_sockaddr_t iface;
-	int tid;
+	int tid = isc_tid();
 	uint32_t initial = 0;
 
-	if (result != ISC_R_SUCCESS) {
-		return (result);
-	}
-
-	INSIST(VALID_NMHANDLE(handle));
-	INSIST(VALID_NMSOCK(handle->sock));
-	INSIST(VALID_NMSOCK(listensock));
-	INSIST(listensock->type == isc_nm_streamdnslistener);
+	REQUIRE(VALID_NMHANDLE(handle));
+	REQUIRE(VALID_NMSOCK(handle->sock));
 
 	if (isc__nm_closing(handle->sock->worker)) {
 		return (ISC_R_SHUTTINGDOWN);
-	} else if (isc__nmsocket_closing(handle->sock) || listensock->closing) {
-		return (ISC_R_CANCELED);
+	} else if (result != ISC_R_SUCCESS) {
+		return (result);
 	}
 
-	tid = isc_tid();
+	REQUIRE(VALID_NMSOCK(listensock));
+	REQUIRE(listensock->type == isc_nm_streamdnslistener);
+
 	iface = isc_nmhandle_localaddr(handle);
 	nsock = streamdns_sock_new(handle->sock->worker, isc_nm_streamdnssocket,
 				   &iface, true);
@@ -675,7 +671,7 @@ streamdns_accept_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 	nsock->accepting = true;
 	nsock->active = true;
 
-	isc__nmsocket_attach(listensock, &nsock->listener);
+	isc__nmsocket_attach(handle->sock, &nsock->listener);
 	isc_nmhandle_attach(handle, &nsock->outerhandle);
 	handle->sock->streamdns.sock = nsock;
 
@@ -753,10 +749,8 @@ isc_nm_listenstreamdns(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
 
 	listener->result = result;
 	listener->active = true;
-	listener->listening = true;
 	INSIST(listener->outer->streamdns.listener == NULL);
 	listener->nchildren = listener->outer->nchildren;
-	isc__nmsocket_barrier_init(listener);
 	isc__nmsocket_attach(listener, &listener->outer->streamdns.listener);
 
 	*sockp = listener;

lib/isc/netmgr/tcp.c

@@ -415,8 +415,6 @@ start_tcp_child_job(void *arg) {
 		goto done;
 	}
 
-	sock->listening = true;
-
 	if (sock->tid == 0) {
 		r = uv_tcp_getsockname(&sock->uv_handle.tcp,
 				       (struct sockaddr *)&ss,
@@ -675,7 +673,6 @@ tcp_stop_cb(uv_handle_t *handle) {
 	REQUIRE(!sock->closed);
 
 	sock->closed = true;
-	sock->listening = false;
 
 	isc__nm_incstats(sock, STATID_CLOSE);
 

lib/isc/netmgr/tlsstream.c

@@ -111,8 +111,6 @@ inactive(isc_nmsocket_t *sock) {
 		sock->outerhandle == NULL ||
 		!isc__nmsocket_active(sock->outerhandle->sock) ||
 		sock->outerhandle->sock->closing ||
-		(sock->listener != NULL &&
-		 !isc__nmsocket_active(sock->listener)) ||
 		isc__nm_closing(sock->worker));
 }
 
@@ -384,53 +382,42 @@ tls_process_outgoing(isc_nmsocket_t *sock, bool finish,
 
 static int
 tls_try_handshake(isc_nmsocket_t *sock, isc_result_t *presult) {
-	int rv = 0;
-	isc_nmhandle_t *tlshandle = NULL;
-
 	REQUIRE(sock->tlsstream.state == TLS_HANDSHAKE);
 
 	if (SSL_is_init_finished(sock->tlsstream.tls) == 1) {
 		return (0);
 	}
 
-	rv = SSL_do_handshake(sock->tlsstream.tls);
+	int rv = SSL_do_handshake(sock->tlsstream.tls);
 	if (rv == 1) {
+		isc_nmhandle_t *tlshandle = NULL;
 		isc_result_t result = ISC_R_SUCCESS;
+
+		REQUIRE(sock->statichandle == NULL);
 		INSIST(SSL_is_init_finished(sock->tlsstream.tls) == 1);
-		INSIST(sock->statichandle == NULL);
+
 		isc__nmsocket_log_tls_session_reuse(sock, sock->tlsstream.tls);
 		tlshandle = isc__nmhandle_get(sock, &sock->peer, &sock->iface);
 		tls_read_stop(sock);
+
+		if (isc__nm_closing(sock->worker)) {
+			result = ISC_R_SHUTTINGDOWN;
+		}
+
 		if (sock->tlsstream.server) {
 			/*
-			 * We need to check for 'sock->listener->closing' to
-			 * make sure that we are not breaking the contract by
-			 * calling an accept callback after the listener socket
-			 * was shot down. Also, in this case the accept callback
-			 * can be 'NULL'. That can happen as calling the accept
-			 * callback in TLS is deferred until handshake is done.
-			 * There is a possibility for that to happen *after* the
-			 * underlying TCP connection was accepted. That is, a
-			 * situation possible when the underlying TCP connection
-			 * was accepted, handshake related data transmission
-			 * took place, but in the middle of that the socket is
-			 * being shot down before the TLS accept callback could
-			 * have been called.
+			 * The listening sockets are now closed from outer
+			 * to inner order, which means that this function
+			 * will never be called when the outer socket has
+			 * stopped listening.
 			 *
 			 * Also see 'isc__nmsocket_stop()' - the function used
 			 * to shut down the listening TLS socket - for more
 			 * details.
 			 */
-			if (isc__nm_closing(sock->worker)) {
-				result = ISC_R_SHUTTINGDOWN;
-			} else if (isc__nmsocket_closing(sock) ||
-				   sock->listener->closing)
-			{
-				result = ISC_R_CANCELED;
-			} else {
-				result = sock->listener->accept_cb(
-					tlshandle, result,
-					sock->listener->accept_cbarg);
+			if (result == ISC_R_SUCCESS) {
+				result = sock->accept_cb(tlshandle, result,
+							 sock->accept_cbarg);
 			}
 		} else {
 			tls_call_connect_cb(sock, tlshandle, result);
@@ -873,7 +860,9 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 		return (ISC_R_TLSERROR);
 	}
 
-	isc__nmsocket_attach(tlslistensock, &tlssock->listener);
+	tlssock->accept_cb = tlslistensock->accept_cb;
+	tlssock->accept_cbarg = tlslistensock->accept_cbarg;
+	isc__nmsocket_attach(handle->sock, &tlssock->listener);
 	isc_nmhandle_attach(handle, &tlssock->outerhandle);
 	tlssock->peer = handle->sock->peer;
 	tlssock->read_timeout =
@@ -952,10 +941,7 @@ isc_nm_listentls(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
 	INSIST(result != ISC_R_UNSET);
 	tlssock->nchildren = tlssock->outer->nchildren;
 
-	isc__nmsocket_barrier_init(tlssock);
-
 	if (result == ISC_R_SUCCESS) {
-		tlssock->listening = true;
 		*sockp = tlssock;
 	}
 

lib/isc/netmgr/udp.c

@@ -168,8 +168,6 @@ start_udp_child_job(void *arg) {
 	}
 
 	sock->reading = true;
-	sock->listening = true;
-
 done:
 	result = isc_uverr2result(r);
 
@@ -951,9 +949,6 @@ udp_close_cb(uv_handle_t *handle) {
 		isc__nmsocket_detach(&sock->server);
 	}
 
-	/* All sockets */
-	sock->listening = false;
-
 	if (sock->parent != NULL) {
 		/* listening socket (listen) */
 		isc__nmsocket_detach(&sock);