bin/dnssec/dnssec-signzone.c: Protect global variables by making them atomic

Both global shuttingdown and finished bool variables were prone to data race
(as reported by ThreadSanitizer).  The commit makes them both atomic.

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index f3a24cb1c33c90bbf611e93ed84931a5d680e459..e518ab3720b19b1f632a909f877278811d91dfd5 100644 (file)
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -32,6 +32,7 @@
 #include <unistd.h>
 
 #include <isc/app.h>
+#include <isc/atomic.h>
 #include <isc/base32.h>
 #include <isc/commandline.h>
 #include <isc/event.h>
@@ -45,8 +46,8 @@
 #include <isc/print.h>
 #include <isc/random.h>
 #include <isc/rwlock.h>
-#include <isc/serial.h>
 #include <isc/safe.h>
+#include <isc/serial.h>
 #include <isc/stdio.h>
 #include <isc/string.h>
 #include <isc/task.h>
@@ -155,7 +156,8 @@ static unsigned char *gsalt = saltbuf;
 static size_t salt_length = 0;
 static isc_task_t *master = NULL;
 static unsigned int ntasks = 0;
-static bool shuttingdown = false, finished = false;
+static atomic_bool shuttingdown = ATOMIC_VAR_INIT(false);
+static atomic_bool finished = ATOMIC_VAR_INIT(false);
 static bool nokeys = false;
 static bool removefile = false;
 static bool generateds = false;
@@ -1454,11 +1456,12 @@ signapex(void) {
        cleannode(gdb, gversion, node);
        dns_db_detachnode(gdb, &node);
        result = dns_dbiterator_first(gdbiter);
-       if (result == ISC_R_NOMORE)
-               finished = true;
-       else if (result != ISC_R_SUCCESS)
+       if (result == ISC_R_NOMORE) {
+               atomic_store(&finished, true);
+       } else if (result != ISC_R_SUCCESS) {
                fatal("failure iterating database: %s",
                      isc_result_totext(result));
+       }
 }
 
 /*%
@@ -1478,11 +1481,12 @@ assignwork(isc_task_t *task, isc_task_t *worker) {
        static dns_fixedname_t fzonecut;        /* Protected by namelock. */
        static unsigned int ended = 0;          /* Protected by namelock. */
 
-       if (shuttingdown)
+       if (atomic_load(&shuttingdown)) {
                return;
+       }
 
        LOCK(&namelock);
-       if (finished) {
+       if (atomic_load(&finished)) {
                ended++;
                if (ended == ntasks) {
                        isc_task_detach(&task);
@@ -1552,7 +1556,7 @@ assignwork(isc_task_t *task, isc_task_t *worker) {
  next:
                result = dns_dbiterator_next(gdbiter);
                if (result == ISC_R_NOMORE) {
-                       finished = true;
+                       atomic_store(&finished, true);
                        break;
                } else if (result != ISC_R_SUCCESS)
                        fatal("failure iterating database: %s",
@@ -3854,7 +3858,7 @@ main(int argc, char *argv[]) {
        presign();
        TIME_NOW(&sign_start);
        signapex();
-       if (!finished) {
+       if (!atomic_load(&finished)) {
                /*
                 * There is more work to do.  Spread it out over multiple
                 * processors if possible.
@@ -3867,11 +3871,12 @@ main(int argc, char *argv[]) {
                                      isc_result_totext(result));
                }
                (void)isc_app_run();
-               if (!finished)
+               if (!atomic_load(&finished)) {
                        fatal("process aborted by user");
+               }
        } else
                isc_task_detach(&master);
-       shuttingdown = true;
+       atomic_store(&shuttingdown, true);;
        for (i = 0; i < (int)ntasks; i++)
                isc_task_detach(&tasks[i]);
        isc_taskmgr_destroy(&taskmgr);