4076. [bug] Named could crash on shutdown with outstanding

                        reload / reconfig events. [RT #38622]

diff --git a/CHANGES b/CHANGES
index 5c05e887db67434846404d9dd474a809a889600c..20918958dbdea898930991aeb03d626f469721d0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4076.  [bug]           Named could crash on shutdown with outstanding
+                       reload / reconfig events. [RT #38622]
+
 4075.  [placeholder]
 
 4074.  [cleanup]       Cleaned up more warnings from gcc -Wshadow. [RT #38708]

diff --git a/bin/named/main.c b/bin/named/main.c
index 2ed94f8182e112b12ccc394951cd768841aff6cb..2ff6b84506181a0db022d125589ca01c317b1fd3 100644 (file)
--- a/bin/named/main.c
+++ b/bin/named/main.c
@@ -796,10 +796,6 @@ static void
 destroy_managers(void) {
        ns_lwresd_shutdown();
 
-       isc_entropy_detach(&ns_g_entropy);
-       if (ns_g_fallbackentropy != NULL)
-               isc_entropy_detach(&ns_g_fallbackentropy);
-
        /*
         * isc_taskmgr_destroy() will block until all tasks have exited,
         */
@@ -1128,6 +1124,10 @@ cleanup(void) {
 
        ns_server_destroy(&ns_g_server);
 
+       isc_entropy_detach(&ns_g_entropy);
+       if (ns_g_fallbackentropy != NULL)
+               isc_entropy_detach(&ns_g_fallbackentropy);
+
        ns_builtin_deinit();
 
        /*

diff --git a/bin/named/server.c b/bin/named/server.c
index 0266522aa430213ed4d937bde40b35f71f4f3cc6..4b3d18fc41a0a56182d86d3e40787b23af4088de 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -6365,6 +6365,8 @@ load_configuration(const char *filename, ns_server_t *server,
        if (view != NULL)
                dns_view_detach(&view);
 
+       ISC_LIST_APPENDLIST(viewlist, builtin_viewlist, link);
+
        /*
         * This cleans up either the old production view list
         * or our temporary list depending on whether they

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index 46be4ff19cf2b129e3e3215f5955011cb4b65e82..f235879b5cede83242a73d7a07ef706490d3a4bf 100644 (file)
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -2533,7 +2533,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr,
                              nbuckets[11]);
                adb->nentries = nbuckets[11];
                adb->nnames = nbuckets[11];
-
        }
 
        isc_mem_attach(mem, &adb->mctx);
@@ -2733,6 +2732,8 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr,
  fail0c:
        DESTROYLOCK(&adb->lock);
  fail0b:
+       if (adb->excl != NULL)
+               isc_task_detach(&adb->excl);
        isc_mem_putanddetach(&adb->mctx, adb, sizeof(dns_adb_t));
 
        return (result);

diff --git a/lib/isc/task.c b/lib/isc/task.c
index fff6b884eaf7a75b9f9a03a4713cd57c92a36245..069b5a30830a856cd79f4357b9a38ee6756b66f1 100644 (file)
--- a/lib/isc/task.c
+++ b/lib/isc/task.c
@@ -159,6 +159,13 @@ struct isc__taskmgr {
        isc_boolean_t                   pause_requested;
        isc_boolean_t                   exclusive_requested;
        isc_boolean_t                   exiting;
+
+       /*
+        * Multiple threads can read/write 'excl' at the same time, so we need
+        * to protect the access.  We can't use 'lock' since isc_task_detach()
+        * will try to acquire it.
+        */
+       isc_mutex_t                     excl_lock;
        isc__task_t                     *excl;
 #ifdef USE_SHARED_MANAGER
        unsigned int                    refs;
@@ -1315,6 +1322,7 @@ manager_free(isc__taskmgr_t *manager) {
        isc_mem_free(manager->mctx, manager->threads);
 #endif /* USE_WORKER_THREADS */
        DESTROYLOCK(&manager->lock);
+       DESTROYLOCK(&manager->excl_lock);
        manager->common.impmagic = 0;
        manager->common.magic = 0;
        mctx = manager->mctx;
@@ -1367,6 +1375,11 @@ isc__taskmgr_create(isc_mem_t *mctx, unsigned int workers,
        result = isc_mutex_init(&manager->lock);
        if (result != ISC_R_SUCCESS)
                goto cleanup_mgr;
+       result = isc_mutex_init(&manager->excl_lock);
+       if (result != ISC_R_SUCCESS) {
+               DESTROYLOCK(&manager->lock);
+               goto cleanup_mgr;
+       }
 
 #ifdef USE_WORKER_THREADS
        manager->workers = 0;
@@ -1499,8 +1512,10 @@ isc__taskmgr_destroy(isc_taskmgr_t **managerp) {
        /*
         * Detach the exclusive task before acquiring the manager lock
         */
+       LOCK(&manager->excl_lock);
        if (manager->excl != NULL)
                isc__task_detach((isc_task_t **) &manager->excl);
+       UNLOCK(&manager->excl_lock);
 
        /*
         * Unlike elsewhere, we're going to hold this lock a long time.
@@ -1657,23 +1672,29 @@ isc_taskmgr_setexcltask(isc_taskmgr_t *mgr0, isc_task_t *task0) {
 
        REQUIRE(VALID_MANAGER(mgr));
        REQUIRE(VALID_TASK(task));
+       LOCK(&mgr->excl_lock);
        if (mgr->excl != NULL)
                isc__task_detach((isc_task_t **) &mgr->excl);
        isc__task_attach(task0, (isc_task_t **) &mgr->excl);
+       UNLOCK(&mgr->excl_lock);
 }
 
 isc_result_t
 isc_taskmgr_excltask(isc_taskmgr_t *mgr0, isc_task_t **taskp) {
        isc__taskmgr_t *mgr = (isc__taskmgr_t *) mgr0;
+       isc_result_t result = ISC_R_SUCCESS;
 
        REQUIRE(VALID_MANAGER(mgr));
        REQUIRE(taskp != NULL && *taskp == NULL);
 
-       if (mgr->excl == NULL)
-               return (ISC_R_NOTFOUND);
+       LOCK(&mgr->excl_lock);
+       if (mgr->excl != NULL)
+               isc__task_attach((isc_task_t *) mgr->excl, taskp);
+       else
+               result = ISC_R_NOTFOUND;
+       UNLOCK(&mgr->excl_lock);
 
-       isc__task_attach((isc_task_t *) mgr->excl, taskp);
-       return (ISC_R_SUCCESS);
+       return (result);
 }
 
 isc_result_t