diff --git a/CHANGES b/CHANGES index cd12176c6e..8023c4855b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6120. [bug] Use two pairs of dns_db_t and dns_dbversion_t in a + catalog zone structure to avoid a race between the + dns__catz_update_cb() and dns_catz_dbupdate_callback() + functions. [GL #3907] + 6119. [bug] Make sure to revert the reconfigured zones to the previous version of the view, when the new view reconfiguration fails during the configuration of @@ -16,8 +21,7 @@ 6115. [bug] Unregister db update notify callback before detaching from the previous db inside the catz update notify - callback. Also, call the db notify callbacks only after - zone_postload() returns successfully. [GL #3777] + callback. [GL #3777] 6114. [func] Run the catalog zone update process on the offload threads. [GL #3881] diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 3a28fbcf0e..73d30b62a0 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -88,11 +88,13 @@ struct dns_catz_zone { dns_catz_options_t zoneoptions; isc_time_t lastupdated; - bool updatepending; /* there is an update pending */ - bool updaterunning; /* there is an update running */ - isc_result_t updateresult; /* result from the offloaded work */ - dns_db_t *db; /* zones database */ - dns_dbversion_t *dbversion; /* zones database version */ + bool updatepending; /* there is an update pending */ + bool updaterunning; /* there is an update running */ + isc_result_t updateresult; /* result from the offloaded work */ + dns_db_t *db; /* zones database */ + dns_dbversion_t *dbversion; /* version we will be updating to */ + dns_db_t *updb; /* zones database we're working on */ + dns_dbversion_t *updbversion; /* version we're working on */ isc_timer_t *updatetimer; @@ -2095,12 +2097,29 @@ dns__catz_timer_cb(void *arg) { INSIST(DNS_DB_VALID(catz->db)); INSIST(catz->dbversion != NULL); + INSIST(catz->updb == NULL); + INSIST(catz->updbversion == NULL); catz->updatepending = false; catz->updaterunning = true; catz->updateresult = ISC_R_UNSET; dns_name_format(&catz->name, domain, DNS_NAME_FORMATSIZE); + + if (!catz->active) { + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_MASTER, ISC_LOG_INFO, + "catz: %s: no longer active, reload is canceled", + domain); + catz->updaterunning = false; + catz->updateresult = ISC_R_CANCELED; + goto exit; + } + + dns_db_attach(catz->db, &catz->updb); + catz->updbversion = catz->dbversion; + catz->dbversion = NULL; + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_INFO, "catz: %s: reload start", domain); @@ -2108,6 +2127,7 @@ dns__catz_timer_cb(void *arg) { isc_work_enqueue(catz->loop, dns__catz_update_cb, dns__catz_done_cb, catz); +exit: isc_timer_destroy(&catz->updatetimer); catz->loop = NULL; @@ -2193,14 +2213,14 @@ catz_rdatatype_is_processable(const dns_rdatatype_t type) { static void dns__catz_update_cb(void *data) { dns_catz_zone_t *catz = (dns_catz_zone_t *)data; - dns_db_t *db = NULL; + dns_db_t *updb = NULL; dns_catz_zones_t *catzs = NULL; dns_catz_zone_t *oldcatz = NULL, *newcatz = NULL; isc_result_t result; isc_region_t r; dns_dbnode_t *node = NULL; const dns_dbnode_t *vers_node = NULL; - dns_dbiterator_t *it = NULL; + dns_dbiterator_t *updbit = NULL; dns_fixedname_t fixname; dns_name_t *name = NULL; dns_rdatasetiter_t *rdsiter = NULL; @@ -2208,6 +2228,7 @@ dns__catz_update_cb(void *data) { char bname[DNS_NAME_FORMATSIZE]; char cname[DNS_NAME_FORMATSIZE]; bool is_vers_processed = false; + bool is_active; uint32_t vers; uint32_t catz_vers; @@ -2215,7 +2236,7 @@ dns__catz_update_cb(void *data) { REQUIRE(DNS_DB_VALID(catz->db)); REQUIRE(DNS_CATZ_ZONES_VALID(catz->catzs)); - db = catz->db; + updb = catz->updb; catzs = catz->catzs; if (atomic_load(&catzs->shuttingdown)) { @@ -2223,14 +2244,15 @@ dns__catz_update_cb(void *data) { goto exit; } - dns_name_format(&db->origin, bname, DNS_NAME_FORMATSIZE); + dns_name_format(&updb->origin, bname, DNS_NAME_FORMATSIZE); /* * Create a new catz in the same context as current catz. */ - dns_name_toregion(&db->origin, &r); + dns_name_toregion(&updb->origin, &r); LOCK(&catzs->lock); result = isc_ht_find(catzs->zones, r.base, r.length, (void **)&oldcatz); + is_active = (result == ISC_R_SUCCESS && oldcatz->active); UNLOCK(&catzs->lock); if (result != ISC_R_SUCCESS) { /* This can happen if we remove the zone in the meantime. */ @@ -2240,7 +2262,16 @@ dns__catz_update_cb(void *data) { goto exit; } - result = dns_db_getsoaserial(db, oldcatz->dbversion, &vers); + if (!is_active) { + /* This can happen during a reconfiguration. */ + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_MASTER, ISC_LOG_INFO, + "catz: zone '%s' is no longer active", bname); + result = ISC_R_CANCELED; + goto exit; + } + + result = dns_db_getsoaserial(updb, oldcatz->updbversion, &vers); if (result != ISC_R_SUCCESS) { /* A zone without SOA record?!? */ isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -2255,9 +2286,8 @@ dns__catz_update_cb(void *data) { "catz: updating catalog zone '%s' with serial %" PRIu32, bname, vers); - result = dns_catz_new_zone(catzs, &newcatz, &db->origin); + result = dns_catz_new_zone(catzs, &newcatz, &updb->origin); if (result != ISC_R_SUCCESS) { - dns_db_closeversion(db, &oldcatz->dbversion, false); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create new zone - %s", @@ -2265,10 +2295,9 @@ dns__catz_update_cb(void *data) { goto exit; } - result = dns_db_createiterator(db, DNS_DB_NONSEC3, &it); + result = dns_db_createiterator(updb, DNS_DB_NONSEC3, &updbit); if (result != ISC_R_SUCCESS) { dns_catz_detach_catz(&newcatz); - dns_db_closeversion(db, &oldcatz->dbversion, false); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create DB iterator - %s", @@ -2283,21 +2312,19 @@ dns__catz_update_cb(void *data) { * records might be processed differently depending on the version of * the catalog zone's schema. */ - result = dns_name_fromstring2(name, "version", &db->origin, 0, NULL); + result = dns_name_fromstring2(name, "version", &updb->origin, 0, NULL); if (result != ISC_R_SUCCESS) { - dns_dbiterator_destroy(&it); + dns_dbiterator_destroy(&updbit); dns_catz_detach_catz(&newcatz); - dns_db_closeversion(db, &oldcatz->dbversion, false); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to create name from string - %s", isc_result_totext(result)); goto exit; } - result = dns_dbiterator_seek(it, name); + result = dns_dbiterator_seek(updbit, name); if (result != ISC_R_SUCCESS) { - dns_dbiterator_destroy(&it); - dns_db_closeversion(db, &oldcatz->dbversion, false); + dns_dbiterator_destroy(&updbit); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: zone '%s' has no 'version' record (%s)", @@ -2317,7 +2344,7 @@ dns__catz_update_cb(void *data) { break; } - result = dns_dbiterator_current(it, &node, name); + result = dns_dbiterator_current(updbit, &node, name); if (result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, @@ -2326,7 +2353,7 @@ dns__catz_update_cb(void *data) { break; } - result = dns_dbiterator_pause(it); + result = dns_dbiterator_pause(updbit); RUNTIME_CHECK(result == ISC_R_SUCCESS); if (!is_vers_processed) { @@ -2334,19 +2361,19 @@ dns__catz_update_cb(void *data) { vers_node = node; } else if (node == vers_node) { /* Skip the already processed version node */ - dns_db_detachnode(db, &node); - result = dns_dbiterator_next(it); + dns_db_detachnode(updb, &node); + result = dns_dbiterator_next(updbit); continue; } - result = dns_db_allrdatasets(db, node, oldcatz->dbversion, 0, 0, - &rdsiter); + result = dns_db_allrdatasets(updb, node, oldcatz->updbversion, + 0, 0, &rdsiter); if (result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "catz: failed to fetch rrdatasets - %s", isc_result_totext(result)); - dns_db_detachnode(db, &node); + dns_db_detachnode(updb, &node); break; } @@ -2401,18 +2428,17 @@ dns__catz_update_cb(void *data) { dns_rdatasetiter_destroy(&rdsiter); - dns_db_detachnode(db, &node); + dns_db_detachnode(updb, &node); if (!is_vers_processed) { is_vers_processed = true; - result = dns_dbiterator_first(it); + result = dns_dbiterator_first(updbit); } else { - result = dns_dbiterator_next(it); + result = dns_dbiterator_next(updbit); } } - dns_dbiterator_destroy(&it); - dns_db_closeversion(db, &oldcatz->dbversion, false); + dns_dbiterator_destroy(&updbit); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), "catz: update_from_db: iteration finished: %s", @@ -2477,13 +2503,15 @@ final: * update callback in zone_startload or axfr_makedb, but we will * call onupdate() artificially so we can register the callback here. */ + LOCK(&catzs->lock); if (!oldcatz->db_registered) { result = dns_db_updatenotify_register( - db, dns_catz_dbupdate_callback, oldcatz->catzs); + updb, dns_catz_dbupdate_callback, oldcatz->catzs); if (result == ISC_R_SUCCESS) { oldcatz->db_registered = true; } } + UNLOCK(&catzs->lock); exit: catz->updateresult = result; @@ -2506,6 +2534,9 @@ dns__catz_done_cb(void *data) { dns__catz_timer_start(catz); } + dns_db_closeversion(catz->updb, &catz->updbversion, false); + dns_db_detach(&catz->updb); + UNLOCK(&catz->catzs->lock); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, @@ -2522,6 +2553,7 @@ dns_catz_prereconfig(dns_catz_zones_t *catzs) { REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); + LOCK(&catzs->lock); isc_ht_iter_create(catzs->zones, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS; result = isc_ht_iter_next(iter)) @@ -2530,6 +2562,7 @@ dns_catz_prereconfig(dns_catz_zones_t *catzs) { isc_ht_iter_current(iter, (void **)&catz); catz->active = false; } + UNLOCK(&catzs->lock); INSIST(result == ISC_R_NOMORE); isc_ht_iter_destroy(&iter); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 544b502a4c..bb3bcaa79b 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -17244,6 +17244,13 @@ zone_loaddone(void *arg, isc_result_t result) { dns_zone_catz_disable_db(zone, load->db); } + tresult = dns_db_endload(load->db, &load->callbacks); + if (tresult != ISC_R_SUCCESS && + (result == ISC_R_SUCCESS || result == DNS_R_SEENINCLUDE)) + { + result = tresult; + } + /* * Lock hierarchy: zmgr, zone, raw. */ @@ -17262,13 +17269,9 @@ again: goto again; } } - tresult = zone_postload(zone, load->db, load->loadtime, result); - if (tresult != ISC_R_SUCCESS && - (result == ISC_R_SUCCESS || result == DNS_R_SEENINCLUDE)) - { - result = tresult; - } + (void)zone_postload(zone, load->db, load->loadtime, result); DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_LOADING); + zone_idetach(&load->callbacks.zone); /* * Leave the zone frozen if the reload fails. */ @@ -17285,12 +17288,6 @@ again: } UNLOCK_ZONE(zone); - (void)dns_db_endload(load->db, &load->callbacks); - - LOCK_ZONE(zone); - zone_idetach(&load->callbacks.zone); - UNLOCK_ZONE(zone); - dns_db_detach(&load->db); if (zone->loadctx != NULL) { dns_loadctx_detach(&zone->loadctx);