Make dns_dt_send() call dns_dt_reopen() asynchronously

Instead of checking current dnstap output file size and potentially
synchronously calling dns_dt_reopen() upon every call to dns_dt_send():

  - call dns_dt_reopen() asynchronously by queuing an event to the task
    specified at dnstap environment creation time,

  - ensure no roll event is outstanding before checking dnstap output
    file size and potentially queuing another roll event.

This causes dnstap output files to exceed their configured size limits,
but prevents any two threads from performing the roll simultaneously
(which causes crashes).

diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c
index 6973a0b2d3ed5eea9e237fb5d5be1d6ae3472f89..6403303aab56683f41cd7ba92f68d259875f9819 100644 (file)
--- a/lib/dns/dnstap.c
+++ b/lib/dns/dnstap.c
@@ -52,6 +52,7 @@
 #include <isc/file.h>
 #include <isc/log.h>
 #include <isc/mem.h>
+#include <isc/mutex.h>
 #include <isc/once.h>
 #include <isc/print.h>
 #include <isc/sockaddr.h>
@@ -62,6 +63,7 @@
 #include <isc/util.h>
 
 #include <dns/dnstap.h>
+#include <dns/events.h>
 #include <dns/log.h>
 #include <dns/message.h>
 #include <dns/name.h>
@@ -103,6 +105,8 @@ struct dns_dtenv {
        struct fstrm_iothr_options *fopt;
 
        isc_task_t *reopen_task;
+       isc_mutex_t reopen_lock;        /* locks 'reopen_queued' */
+       isc_boolean_t reopen_queued;
 
        isc_region_t identity;
        isc_region_t version;
@@ -262,6 +266,8 @@ dns_dt_create2(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
        *foptp = NULL;
 
        env->reopen_task = reopen_task;
+       isc_mutex_init(&env->reopen_lock);
+       env->reopen_queued = ISC_FALSE;
 
        isc_mem_attach(mctx, &env->mctx);
 
@@ -280,6 +286,7 @@ dns_dt_create2(isc_mem_t *mctx, dns_dtmode_t mode, const char *path,
 
        if (result != ISC_R_SUCCESS) {
                if (env != NULL) {
+                       isc_mutex_destroy(&env->reopen_lock);
                        if (env->mctx != NULL)
                                isc_mem_detach(&env->mctx);
                        if (env->path != NULL)
@@ -737,6 +744,91 @@ setaddr(dns_dtmsg_t *dm, isc_sockaddr_t *sa, isc_boolean_t tcp,
        *has_port = 1;
 }
 
+/*%
+ * Invoke dns_dt_reopen() and re-allow dnstap output file rolling.  This
+ * function is run in the context of the task stored in the 'reopen_task' field
+ * of the dnstap environment structure.
+ */
+static void
+perform_reopen(isc_task_t *task, isc_event_t *event) {
+       dns_dtenv_t *env;
+
+       REQUIRE(event != NULL);
+       REQUIRE(event->ev_type == DNS_EVENT_FREESTORAGE);
+
+       env = (dns_dtenv_t *)event->ev_arg;
+
+       REQUIRE(VALID_DTENV(env));
+       REQUIRE(task == env->reopen_task);
+
+       /*
+        * Roll output file in the context of env->reopen_task.
+        */
+       dns_dt_reopen(env, env->rolls);
+
+       /*
+        * Clean up.
+        */
+       isc_event_free(&event);
+       isc_task_detach(&task);
+
+       /*
+        * Re-allow output file rolling.
+        */
+       LOCK(&env->reopen_lock);
+       env->reopen_queued = ISC_FALSE;
+       UNLOCK(&env->reopen_lock);
+}
+
+/*%
+ * Check whether a dnstap output file roll is due and if so, initiate it (the
+ * actual roll happens asynchronously).
+ */
+static void
+check_file_size_and_maybe_reopen(dns_dtenv_t *env) {
+       isc_task_t *reopen_task = NULL;
+       isc_event_t *event;
+       struct stat statbuf;
+
+       /*
+        * If the task from which the output file should be reopened was not
+        * specified, abort.
+        */
+       if (env->reopen_task == NULL) {
+               return;
+       }
+
+       /*
+        * If an output file roll is not currently queued, check the current
+        * size of the output file to see whether a roll is needed.  Return if
+        * it is not.
+        */
+       LOCK(&env->reopen_lock);
+       if (env->reopen_queued || stat(env->path, &statbuf) < 0 ||
+           statbuf.st_size <= env->max_size)
+       {
+               goto unlock_and_return;
+       }
+
+       /*
+        * We need to roll the output file, but it needs to be done in the
+        * context of env->reopen_task.  Allocate and send an event to achieve
+        * that, then disallow output file rolling until the roll we queue is
+        * completed.
+        */
+       event = isc_event_allocate(env->mctx, NULL, DNS_EVENT_FREESTORAGE,
+                                  perform_reopen, env, sizeof(*event));
+       if (event == NULL) {
+               goto unlock_and_return;
+       }
+       isc_task_attach(env->reopen_task, &reopen_task);
+       isc_task_send(reopen_task, &event);
+       env->reopen_queued = ISC_TRUE;
+
+ unlock_and_return:
+       UNLOCK(&env->reopen_lock);
+}
+
 void
 dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype,
            isc_sockaddr_t *qaddr, isc_sockaddr_t *raddr,
@@ -757,12 +849,7 @@ dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype,
        REQUIRE(VALID_DTENV(view->dtenv));
 
        if (view->dtenv->max_size != 0) {
-               struct stat statbuf;
-               if (stat(view->dtenv->path, &statbuf) >= 0 &&
-                   statbuf.st_size > view->dtenv->max_size)
-               {
-                       dns_dt_reopen(view->dtenv, view->dtenv->rolls);
-               }
+               check_file_size_and_maybe_reopen(view->dtenv);
        }
 
        TIME_NOW(&now);