Refactor the dns_dt API to use ISC_THREAD_LOCAL

Previously, the dns_dt API used isc_thread_key API for TLS, which is
fairly complicated and requires initialization of memory contexts, etc.
This part of code was refactored to use a ISC_THREAD_LOCAL pointer which
greatly simplifies the whole code related to storing TLS variables.

bin/named/server.c

@@ -9742,9 +9742,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) {
 		dns_tsigkey_detach(&named_g_sessionkey);
 		dns_name_free(&named_g_sessionkeyname, server->mctx);
 	}
-#ifdef HAVE_DNSTAP
-	dns_dt_shutdown();
-#endif
 #if defined(HAVE_GEOIP2)
 	named_geoip_shutdown();
 	dns_geoip_shutdown();

lib/dns/dnstap.c

@@ -127,59 +127,16 @@ struct dns_dtenv {
 		goto cleanup; \
 	} while (0)
 
-static isc_mutex_t dt_mutex;
-static bool dt_initialized = false;
-static isc_thread_key_t dt_key;
-static isc_once_t mutex_once = ISC_ONCE_INIT;
-static isc_mem_t *dt_mctx = NULL;
 
-/*
- * Change under task exclusive.
- */
-static unsigned int generation;
+typedef struct ioq {
+	unsigned int generation;
+	struct fstrm_iothr_queue *ioq;
+} dt__ioq_t;
 
-static void
-mutex_init(void) {
-	isc_mutex_init(&dt_mutex);
-}
 
-static void
-dtfree(void *arg) {
-	free(arg);
-	isc_thread_key_setspecific(dt_key, NULL);
-}
+ISC_THREAD_LOCAL dt__ioq_t dt_ioq = { 0 };
 
-static isc_result_t
-dt_init(void) {
-	isc_result_t result;
-
-	result = isc_once_do(&mutex_once, mutex_init);
-	if (result != ISC_R_SUCCESS)
-		return (result);
-
-	if (dt_initialized)
-		return (ISC_R_SUCCESS);
-
-	LOCK(&dt_mutex);
-	if (!dt_initialized) {
-		int ret;
-
-		if (dt_mctx == NULL) {
-			isc_mem_create(&dt_mctx);
-		}
-		isc_mem_setname(dt_mctx, "dt", NULL);
-		isc_mem_setdestroycheck(dt_mctx, false);
-
-		ret = isc_thread_key_create(&dt_key, dtfree);
-		if (ret == 0)
-			dt_initialized = true;
-		else
-			result = ISC_R_FAILURE;
-	}
-	UNLOCK(&dt_mutex);
-
-	return (result);
-}
+static atomic_uint_fast32_t global_generation;
 
 isc_result_t
 dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
@@ -202,7 +159,7 @@ dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
 		      DNS_LOGMODULE_DNSTAP, ISC_LOG_INFO,
 		      "opening dnstap destination '%s'", path);
 
-	generation++;
+	atomic_fetch_add_release(&global_generation, 1);
 
 	env = isc_mem_get(mctx, sizeof(dns_dtenv_t));
 
@@ -380,7 +337,7 @@ dns_dt_reopen(dns_dtenv_t *env, int roll) {
 		      (roll < 0) ? "reopening" : "rolling",
 		      env->path);
 
-	generation++;
+	atomic_fetch_add_release(&global_generation, 1);
 
 	if (env->iothr != NULL) {
 		fstrm_iothr_destroy(&env->iothr);
@@ -474,49 +431,33 @@ dns_dt_setversion(dns_dtenv_t *env, const char *version) {
 	return (toregion(env, &env->version, version));
 }
 
+static void
+set_dt_ioq(unsigned int generation, struct fstrm_iothr_queue *ioq) {
+	dt_ioq.generation = generation;
+	dt_ioq.ioq = ioq;
+}
+
 static struct fstrm_iothr_queue *
 dt_queue(dns_dtenv_t *env) {
-	isc_result_t result;
-	struct ioq {
-		unsigned int generation;
-		struct fstrm_iothr_queue *ioq;
-	} *ioq;
-
 	REQUIRE(VALID_DTENV(env));
 
-	if (env->iothr == NULL)
-		return (NULL);
+	unsigned int generation;
 
-	result = dt_init();
-	if (result != ISC_R_SUCCESS)
+	if (env->iothr == NULL) {
 		return (NULL);
-
-	ioq = (struct ioq *)isc_thread_key_getspecific(dt_key);
-	if (ioq != NULL && ioq->generation != generation) {
-		result = isc_thread_key_setspecific(dt_key, NULL);
-		if (result != ISC_R_SUCCESS)
-			return (NULL);
-		free(ioq);
-		ioq = NULL;
-	}
-	if (ioq == NULL) {
-		ioq = malloc(sizeof(*ioq));
-		if (ioq == NULL)
-			return (NULL);
-		ioq->generation = generation;
-		ioq->ioq = fstrm_iothr_get_input_queue(env->iothr);
-		if (ioq->ioq == NULL) {
-			free(ioq);
-			return (NULL);
-		}
-		result = isc_thread_key_setspecific(dt_key, ioq);
-		if (result != ISC_R_SUCCESS) {
-			free(ioq);
-			return (NULL);
-		}
 	}
 
-	return (ioq->ioq);
+	generation = atomic_load_acquire(&global_generation);
+	if (dt_ioq.ioq != NULL && dt_ioq.generation != generation) {
+		set_dt_ioq(0, NULL);
+	}
+	if (dt_ioq.ioq == NULL) {
+		struct fstrm_iothr_queue *ioq =
+			fstrm_iothr_get_input_queue(env->iothr);
+		set_dt_ioq(generation, ioq);
+	}
+
+	return (dt_ioq.ioq);
 }
 
 void
@@ -547,7 +488,7 @@ destroy(dns_dtenv_t *env) {
 		      "closing dnstap");
 	env->magic = 0;
 
-	generation++;
+	atomic_fetch_add(&global_generation, 1);
 
 	if (env->iothr != NULL)
 		fstrm_iothr_destroy(&env->iothr);
@@ -927,12 +868,6 @@ dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype,
 		send_dt(view->dtenv, dm.buf, dm.len);
 }
 
-void
-dns_dt_shutdown() {
-	if (dt_mctx != NULL)
-		isc_mem_detach(&dt_mctx);
-}
-
 static isc_result_t
 putstr(isc_buffer_t **b, const char *str) {
 	isc_result_t result;

lib/dns/include/dns/dnstap.h

@@ -267,13 +267,6 @@ dns_dt_getstats(dns_dtenv_t *env, isc_stats_t **statsp);
  *\li	ISC_R_NOTFOUND
  */
 
-void
-dns_dt_shutdown(void);
-/*%<
- * Shuts down dnstap and frees global resources. This function must only
- * be called immediately before server shutdown.
- */
-
 void
 dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype,
 	    isc_sockaddr_t *qaddr, isc_sockaddr_t *dstaddr,

lib/dns/tests/dnstap_test.c

@@ -132,8 +132,6 @@ create_test(void **state) {
 	}
 
 	cleanup();
-
-	dns_dt_shutdown();
 }
 
 /* send dnstap messages */
@@ -263,7 +261,6 @@ send_test(void **state) {
 
 	dns_dt_detach(&view->dtenv);
 	dns_dt_detach(&dtenv);
-	dns_dt_shutdown();
 	dns_view_detach(&view);
 
 	result = dns_dt_open(TAPFILE, dns_dtmode_file, dt_mctx, &handle);

lib/dns/win32/libdns.def.in

@@ -354,7 +354,6 @@ dns_dt_send
 dns_dt_setidentity
 dns_dt_setupfile
 dns_dt_setversion
-dns_dt_shutdown
 dns_dtdata_free
 @END NOTYET
 dns_dumpctx_attach