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.

diff --git a/bin/named/server.c b/bin/named/server.c
index 9321bc1dadcb30dc8ed403365dca3ff9b7fc63c8..400bd15b46a2f65db67d1286555ceecee289a0ec 100644 (file)
--- a/bin/named/server.c
+++ b/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();

diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c
index 4e7d8e7b4817b5681cac6a8bbf19a95b9650c279..1f66035ce65ead9492b5ffe710fe4e00b3c27b4b 100644 (file)
--- a/lib/dns/dnstap.c
+++ b/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;
-
-static void
-mutex_init(void) {
-       isc_mutex_init(&dt_mutex);
-}
-
-static void
-dtfree(void *arg) {
-       free(arg);
-       isc_thread_key_setspecific(dt_key, NULL);
-}
-
-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);
+typedef struct ioq {
+       unsigned int generation;
+       struct fstrm_iothr_queue *ioq;
+} dt__ioq_t;
 
-       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);
+ISC_THREAD_LOCAL dt__ioq_t dt_ioq = { 0 };
 
-               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;
+       generation = atomic_load_acquire(&global_generation);
+       if (dt_ioq.ioq != NULL && dt_ioq.generation != generation) {
+               set_dt_ioq(0, 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);
-               }
+       if (dt_ioq.ioq == NULL) {
+               struct fstrm_iothr_queue *ioq =
+                       fstrm_iothr_get_input_queue(env->iothr);
+               set_dt_ioq(generation, ioq);
        }
 
-       return (ioq->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;

diff --git a/lib/dns/include/dns/dnstap.h b/lib/dns/include/dns/dnstap.h
index 2e948ebe2474628f4af4f0a11800e6d3ceb859dc..601d3dabbee62e5c7541ecc8d48d1805664c24fe 100644 (file)
--- a/lib/dns/include/dns/dnstap.h
+++ b/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,

diff --git a/lib/dns/tests/dnstap_test.c b/lib/dns/tests/dnstap_test.c
index 963f290e71ff9f06f3a6419fb551f1d0e07882e9..3c5d407cc109aac21f325a8aefa61fe11c40d9bc 100644 (file)
--- a/lib/dns/tests/dnstap_test.c
+++ b/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);

diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 2eef22497c10bd0939a36a429be587844ffb99c5..f90ef4111328aeba8920b580dc1a198f7b579d51 100644 (file)
--- a/lib/dns/win32/libdns.def.in
+++ b/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