From f16ab01da5d990fea470fa2a56fe11b0faefc93e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 27 Feb 2015 12:34:43 +1100 Subject: [PATCH] 4076. [bug] Named could crash on shutdown with outstanding reload / reconfig events. [RT #38622] (cherry picked from commit bb5df338d9b119bb2fe18dea9b0e3034c3925f7b) --- CHANGES | 3 +++ bin/named/main.c | 8 ++++---- bin/named/server.c | 2 ++ lib/dns/adb.c | 3 ++- lib/isc/task.c | 29 +++++++++++++++++++++++++---- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/CHANGES b/CHANGES index cdcef94495..093334c7ad 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4076. [bug] Named could crash on shutdown with outstanding + reload / reconfig events. [RT #38622] + 4075. [bug] Increase nsupdate's input buffer to accomodate very large RRs. [RT #38689] diff --git a/bin/named/main.c b/bin/named/main.c index fd6cdbe5f2..5664e6545e 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -759,10 +759,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, */ @@ -1029,6 +1025,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 198e65aacb..7011836361 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -5784,6 +5784,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 da77bb6c92..c1a1d750cc 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2474,7 +2474,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); @@ -2667,6 +2666,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 02d6e2abaf..754443385b 100644 --- a/lib/isc/task.c +++ b/lib/isc/task.c @@ -156,6 +156,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; @@ -1307,6 +1314,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; @@ -1359,6 +1367,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; @@ -1490,8 +1503,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. @@ -1650,23 +1665,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_TASKFUNC_SCOPE 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_TASKFUNC_SCOPE isc_result_t