Fix the data race when read-writing sock->active by using cmpxchg

lib/isc/include/isc/netmgr.h

@@ -208,7 +208,7 @@ isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg);
  * is data to process.
  */
 
-isc_result_t
+void
 isc_nm_pauseread(isc_nmhandle_t *handle);
 /*%<
  * Pause reading on this handle's socket, but remember the callback.

lib/isc/netmgr/netmgr-int.h

@@ -660,6 +660,18 @@ isc__nmsocket_active(isc_nmsocket_t *sock);
  * or, for child sockets, 'sock->parent->active'.
  */
 
+bool
+isc__nmsocket_deactivate(isc_nmsocket_t *sock);
+/*%<
+ * @brief Deactivate active socket
+ *
+ * Atomically deactive the socket by setting @p sock->active or, for child
+ * sockets, @p sock->parent->active to @c false
+ *
+ * @param[in] sock - valid nmsocket
+ * @return @c false if the socket was already inactive, @c true otherwise
+ */
+
 void
 isc__nmsocket_clearcb(isc_nmsocket_t *sock);
 /*%<
@@ -718,7 +730,7 @@ isc__nm_tcp_close(isc_nmsocket_t *sock);
 /*%<
  * Close a TCP socket.
  */
-isc_result_t
+void
 isc__nm_tcp_pauseread(isc_nmsocket_t *sock);
 /*%<
  * Pause reading on this socket, while still remembering the callback.

lib/isc/netmgr/netmgr.c

@@ -731,6 +731,19 @@ isc__nmsocket_active(isc_nmsocket_t *sock) {
 	return (atomic_load(&sock->active));
 }
 
+bool
+isc__nmsocket_deactivate(isc_nmsocket_t *sock) {
+	REQUIRE(VALID_NMSOCK(sock));
+
+	if (sock->parent != NULL) {
+		return (atomic_compare_exchange_strong(&sock->parent->active,
+						       &(bool){ true }, false));
+	}
+
+	return (atomic_compare_exchange_strong(&sock->active, &(bool){ true },
+					       false));
+}
+
 void
 isc__nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target) {
 	REQUIRE(VALID_NMSOCK(sock));
@@ -1380,7 +1393,7 @@ isc__nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NM(mgr));
 	REQUIRE(VALID_NMSOCK(sock));
 
-	if (sock != NULL && atomic_load(&sock->active)) {
+	if (sock != NULL && isc__nmsocket_active(sock)) {
 		/* Try to reuse one */
 		req = isc_astack_pop(sock->inactivereqs);
 	}
@@ -1419,7 +1432,7 @@ isc__nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock) {
 	handle = req->handle;
 	req->handle = NULL;
 
-	if (!atomic_load(&sock->active) ||
+	if (!isc__nmsocket_active(sock) ||
 	    !isc_astack_trypush(sock->inactivereqs, req)) {
 		isc_mempool_put(sock->mgr->reqpool, req);
 	}
@@ -1481,7 +1494,7 @@ isc_nm_cancelread(isc_nmhandle_t *handle) {
 	}
 }
 
-isc_result_t
+void
 isc_nm_pauseread(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 
@@ -1489,7 +1502,8 @@ isc_nm_pauseread(isc_nmhandle_t *handle) {
 
 	switch (sock->type) {
 	case isc_nm_tcpsocket:
-		return (isc__nm_tcp_pauseread(sock));
+		isc__nm_tcp_pauseread(sock);
+		break;
 	default:
 		INSIST(0);
 		ISC_UNREACHABLE();

lib/isc/netmgr/tcp.c

@@ -722,17 +722,17 @@ isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) {
 	}
 }
 
-isc_result_t
+void
 isc__nm_tcp_pauseread(isc_nmsocket_t *sock) {
 	isc__netievent_pauseread_t *ievent = NULL;
 
 	REQUIRE(VALID_NMSOCK(sock));
 
-	if (atomic_load(&sock->readpaused)) {
+	if (!atomic_compare_exchange_strong(&sock->readpaused, &(bool){ false },
-		return (ISC_R_SUCCESS);
+					    true)) {
+		return;
 	}
 
-	atomic_store(&sock->readpaused, true);
 	ievent = isc__nm_get_ievent(sock->mgr, netievent_tcppauseread);
 	ievent->sock = sock;
 
@@ -745,7 +745,7 @@ isc__nm_tcp_pauseread(isc_nmsocket_t *sock) {
 				       (isc__netievent_t *)ievent);
 	}
 
-	return (ISC_R_SUCCESS);
+	return;
 }
 
 void
@@ -778,12 +778,11 @@ isc__nm_tcp_resumeread(isc_nmsocket_t *sock) {
 		return;
 	}
 
-	if (!atomic_load(&sock->readpaused)) {
+	if (!atomic_compare_exchange_strong(&sock->readpaused, &(bool){ true },
+					    false)) {
 		return;
 	}
 
-	atomic_store(&sock->readpaused, false);
-
 	ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpstartread);
 	ievent->sock = sock;
 
@@ -903,9 +902,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 
 	REQUIRE(VALID_NMSOCK(ssock));
 
-	if (!atomic_load_relaxed(&ssock->active) ||
+	if (!isc__nmsocket_active(ssock) || atomic_load(&ssock->mgr->closing)) {
-	    atomic_load_relaxed(&ssock->mgr->closing))
-	{
 		/* We're closing, bail */
 		if (quota != NULL) {
 			isc_quota_detach(&quota);
@@ -1197,12 +1194,14 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(sock->tid == isc_nm_tid());
 
-	if (!isc__nmsocket_active(sock)) {
+	/*
+	 * If the socket is active, mark it inactive and
+	 * continue. If it isn't active, stop now.
+	 */
+	if (!isc__nmsocket_deactivate(sock)) {
 		return;
 	}
 
-	atomic_store(&sock->active, false);
-
 	if (sock->type == isc_nm_tcpsocket && sock->statichandle != NULL) {
 		failed_read_cb(sock, ISC_R_CANCELED);
 	}

lib/isc/netmgr/tcpdns.c

@@ -544,7 +544,7 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
 	REQUIRE(worker->id == sock->tid);
 	REQUIRE(sock->tid == isc_nm_tid());
 
-	if (atomic_load(&sock->active) && sock->outerhandle != NULL) {
+	if (isc__nmsocket_active(sock) && sock->outerhandle != NULL) {
 		isc_nmhandle_t *sendhandle = NULL;
 		isc_region_t r;
 

lib/isc/netmgr/udp.c

@@ -262,16 +262,12 @@ isc__nm_udp_stoplistening(isc_nmsocket_t *sock) {
 	REQUIRE(sock->type == isc_nm_udplistener);
 
 	/*
-	 * Socket is already closing; there's nothing to do.
+	 * If the socket is active, mark it inactive and
+	 * continue. If it isn't active, stop now.
 	 */
-	if (!isc__nmsocket_active(sock)) {
+	if (!isc__nmsocket_deactivate(sock)) {
 		return;
 	}
-	/*
-	 * Mark it inactive now so that all sends will be ignored
-	 * and we won't try to stop listening again.
-	 */
-	atomic_store(&sock->active, false);
 
 	/*
 	 * If the manager is interlocked, re-enqueue this as an asynchronous