From 26ad166a05a5f791fc66b4f5039a31bc59d0d6ab Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 16 Feb 2024 11:40:26 +1100 Subject: [PATCH 1/3] Use a new memory context when flushing the cache When the cache's memory context was in over memory state when the cache was flushed it resulted in LRU cleaning removing newly entered data in the new cache straight away until the old cache had been destroyed enough to take it out of over memory state. When flushing the cache create a new memory context for the new db to prevent this. (cherry picked from commit 5e77edd07482a69fdf578e7680a495e00ace8b7b) --- bin/named/server.c | 5 +- lib/dns/cache.c | 370 ++++++++++++++++++------------------ lib/dns/include/dns/cache.h | 4 +- tests/libtest/dns.c | 2 +- 4 files changed, 194 insertions(+), 187 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index f6549ab436..97b4118d3b 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4747,14 +4747,11 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ isc_mem_create(&cmctx); isc_mem_setname(cmctx, "cache"); - isc_mem_create(&hmctx); - isc_mem_setname(hmctx, "cache_heap"); - CHECK(dns_cache_create(cmctx, hmctx, named_g_taskmgr, + CHECK(dns_cache_create(cmctx, named_g_taskmgr, named_g_timermgr, view->rdclass, cachename, "rbt", 0, NULL, &cache)); isc_mem_detach(&cmctx); - isc_mem_detach(&hmctx); } nsc = isc_mem_get(mctx, sizeof(*nsc)); nsc->cache = NULL; diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 2a6344e51f..40732e9180 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -129,6 +129,7 @@ struct dns_cache { isc_mutex_t lock; isc_mem_t *mctx; /* Main cache memory */ isc_mem_t *hmctx; /* Heap memory */ + isc_mem_t *tmctx; /* Tree memory */ isc_taskmgr_t *taskmgr; char *name; isc_refcount_t references; @@ -168,22 +169,52 @@ static void water(void *arg, int mark); static isc_result_t -cache_create_db(dns_cache_t *cache, dns_db_t **db) { +cache_create_db(dns_cache_t *cache, dns_db_t **dbp, isc_mem_t **tmctxp, + isc_mem_t **hmctxp) { isc_result_t result; isc_task_t *dbtask = NULL; isc_task_t *prunetask = NULL; + dns_db_t *db = NULL; + isc_mem_t *tmctx = NULL; + isc_mem_t *hmctx = NULL; - result = dns_db_create(cache->mctx, cache->db_type, dns_rootname, + REQUIRE(dbp != NULL && *dbp == NULL); + REQUIRE(tmctxp != NULL && *tmctxp == NULL); + REQUIRE(hmctxp != NULL && *hmctxp == NULL); + + /* + * This will be the cache memory context, which is subject + * to cleaning when the configured memory limits are exceeded. + */ + isc_mem_create(&tmctx); + isc_mem_setname(tmctx, "cache"); + + /* + * This will be passed to RBTDB to use for heaps. This is separate + * from the main cache memory because it can grow quite large under + * heavy load and could otherwise cause the cache to be cleaned too + * aggressively. + */ + isc_mem_create(&hmctx); + isc_mem_setname(hmctx, "cache_heap"); + + if (strcmp(cache->db_type, "rbt") == 0) { + cache->db_argv[0] = (char *)hmctx; + } + result = dns_db_create(tmctx, cache->db_type, dns_rootname, dns_dbtype_cache, cache->rdclass, cache->db_argc, - cache->db_argv, db); + cache->db_argv, &db); if (result != ISC_R_SUCCESS) { - return (result); + goto cleanup_mctx; } - dns_db_setservestalettl(*db, cache->serve_stale_ttl); - dns_db_setservestalerefresh(*db, cache->serve_stale_refresh); + dns_db_setservestalettl(db, cache->serve_stale_ttl); + dns_db_setservestalerefresh(db, cache->serve_stale_refresh); if (cache->taskmgr == NULL) { + *dbp = db; + *tmctxp = tmctx; + *hmctxp = hmctx; return (ISC_R_SUCCESS); } @@ -199,155 +230,28 @@ cache_create_db(dns_cache_t *cache, dns_db_t **db) { } isc_task_setname(prunetask, "cache_prunetask", NULL); - dns_db_settask(*db, dbtask, prunetask); + dns_db_settask(db, dbtask, prunetask); isc_task_detach(&prunetask); isc_task_detach(&dbtask); + *dbp = db; + *tmctxp = tmctx; + *hmctxp = hmctx; + return (ISC_R_SUCCESS); cleanup_dbtask: isc_task_detach(&dbtask); cleanup_db: - dns_db_detach(db); + dns_db_detach(&db); +cleanup_mctx: + isc_mem_detach(&tmctx); + isc_mem_detach(&hmctx); return (result); } -isc_result_t -dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, - isc_timermgr_t *timermgr, dns_rdataclass_t rdclass, - const char *cachename, const char *db_type, - unsigned int db_argc, char **db_argv, dns_cache_t **cachep) { - isc_result_t result; - dns_cache_t *cache; - int i, extra = 0; - - REQUIRE(cachep != NULL); - REQUIRE(*cachep == NULL); - REQUIRE(cmctx != NULL); - REQUIRE(hmctx != NULL); - REQUIRE(taskmgr != NULL || strcmp(db_type, "rbt") != 0); - REQUIRE(cachename != NULL); - - cache = isc_mem_get(cmctx, sizeof(*cache)); - - cache->mctx = cache->hmctx = NULL; - isc_mem_attach(cmctx, &cache->mctx); - isc_mem_attach(hmctx, &cache->hmctx); - - cache->taskmgr = NULL; - if (taskmgr != NULL) { - isc_taskmgr_attach(taskmgr, &cache->taskmgr); - } - - cache->name = NULL; - if (cachename != NULL) { - cache->name = isc_mem_strdup(cmctx, cachename); - } - - isc_mutex_init(&cache->lock); - - isc_refcount_init(&cache->references, 1); - isc_refcount_init(&cache->live_tasks, 1); - cache->rdclass = rdclass; - cache->serve_stale_ttl = 0; - - cache->stats = NULL; - result = isc_stats_create(cmctx, &cache->stats, - dns_cachestatscounter_max); - if (result != ISC_R_SUCCESS) { - goto cleanup_lock; - } - - cache->db_type = isc_mem_strdup(cmctx, db_type); - - /* - * For databases of type "rbt" we pass hmctx to dns_db_create() - * via cache->db_argv, followed by the rest of the arguments in - * db_argv (of which there really shouldn't be any). - */ - if (strcmp(cache->db_type, "rbt") == 0) { - extra = 1; - } - - cache->db_argc = db_argc + extra; - cache->db_argv = NULL; - - if (cache->db_argc != 0) { - cache->db_argv = isc_mem_get(cmctx, - cache->db_argc * sizeof(char *)); - - for (i = 0; i < cache->db_argc; i++) { - cache->db_argv[i] = NULL; - } - - cache->db_argv[0] = (char *)hmctx; - for (i = extra; i < cache->db_argc; i++) { - cache->db_argv[i] = isc_mem_strdup(cmctx, - db_argv[i - extra]); - } - } - - /* - * Create the database - */ - cache->db = NULL; - result = cache_create_db(cache, &cache->db); - if (result != ISC_R_SUCCESS) { - goto cleanup_dbargv; - } - cache->magic = CACHE_MAGIC; - - /* - * RBT-type cache DB has its own mechanism of cache cleaning and doesn't - * need the control of the generic cleaner. - */ - if (strcmp(db_type, "rbt") == 0) { - result = cache_cleaner_init(cache, NULL, NULL, &cache->cleaner); - } else { - result = cache_cleaner_init(cache, taskmgr, timermgr, - &cache->cleaner); - } - if (result != ISC_R_SUCCESS) { - goto cleanup_db; - } - - result = dns_db_setcachestats(cache->db, cache->stats); - if (result != ISC_R_SUCCESS) { - goto cleanup_db; - } - - *cachep = cache; - return (ISC_R_SUCCESS); - -cleanup_db: - dns_db_detach(&cache->db); -cleanup_dbargv: - for (i = extra; i < cache->db_argc; i++) { - if (cache->db_argv[i] != NULL) { - isc_mem_free(cmctx, cache->db_argv[i]); - } - } - if (cache->db_argv != NULL) { - isc_mem_put(cmctx, cache->db_argv, - cache->db_argc * sizeof(char *)); - } - isc_mem_free(cmctx, cache->db_type); - isc_stats_detach(&cache->stats); -cleanup_lock: - isc_mutex_destroy(&cache->lock); - if (cache->name != NULL) { - isc_mem_free(cmctx, cache->name); - } - if (cache->taskmgr != NULL) { - isc_taskmgr_detach(&cache->taskmgr); - } - isc_mem_detach(&cache->hmctx); - isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); - return (result); -} - static void cache_free(dns_cache_t *cache) { REQUIRE(VALID_CACHE(cache)); @@ -355,7 +259,7 @@ cache_free(dns_cache_t *cache) { isc_refcount_destroy(&cache->references); isc_refcount_destroy(&cache->live_tasks); - isc_mem_clearwater(cache->mctx); + isc_mem_clearwater(cache->tmctx); if (cache->cleaner.task != NULL) { isc_task_detach(&cache->cleaner.task); @@ -387,6 +291,7 @@ cache_free(dns_cache_t *cache) { int extra = 0; if (strcmp(cache->db_type, "rbt") == 0) { extra = 1; + cache->db_argv[0] = NULL; } for (int i = extra; i < cache->db_argc; i++) { if (cache->db_argv[i] != NULL) { @@ -416,10 +321,116 @@ cache_free(dns_cache_t *cache) { isc_mutex_destroy(&cache->lock); cache->magic = 0; - isc_mem_detach(&cache->hmctx); + if (cache->hmctx != NULL) { + isc_mem_detach(&cache->hmctx); + } + if (cache->tmctx != NULL) { + isc_mem_detach(&cache->tmctx); + } isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); } +isc_result_t +dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, + isc_timermgr_t *timermgr, dns_rdataclass_t rdclass, + const char *cachename, const char *db_type, + unsigned int db_argc, char **db_argv, dns_cache_t **cachep) { + isc_result_t result; + dns_cache_t *cache; + int i, extra = 0; + + REQUIRE(cachep != NULL); + REQUIRE(*cachep == NULL); + REQUIRE(cmctx != NULL); + REQUIRE(taskmgr != NULL || strcmp(db_type, "rbt") != 0); + REQUIRE(cachename != NULL); + + cache = isc_mem_get(cmctx, sizeof(*cache)); + *cache = (dns_cache_t){ + .db_type = isc_mem_strdup(cmctx, db_type), + .rdclass = rdclass, + .db_argc = db_argc, + .name = cachename == NULL ? NULL + : isc_mem_strdup(cmctx, cachename), + .magic = CACHE_MAGIC, + }; + + isc_mem_attach(cmctx, &cache->mctx); + + if (taskmgr != NULL) { + isc_taskmgr_attach(taskmgr, &cache->taskmgr); + } + + isc_mutex_init(&cache->lock); + isc_refcount_init(&cache->references, 1); + isc_refcount_init(&cache->live_tasks, 1); + + result = isc_stats_create(cmctx, &cache->stats, + dns_cachestatscounter_max); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + /* + * For databases of type "rbt" we pass hmctx to dns_db_create() + * via cache->db_argv, followed by the rest of the arguments in + * db_argv (of which there really shouldn't be any). + */ + if (strcmp(cache->db_type, "rbt") == 0) { + extra = 1; + cache->db_argc++; + } + + if (cache->db_argc != 0) { + cache->db_argv = isc_mem_get(cmctx, + cache->db_argc * sizeof(char *)); + + for (i = 0; i < cache->db_argc; i++) { + cache->db_argv[i] = NULL; + } + + for (i = extra; i < cache->db_argc; i++) { + cache->db_argv[i] = isc_mem_strdup(cmctx, + db_argv[i - extra]); + } + } + + /* + * Create the database + */ + result = cache_create_db(cache, &cache->db, &cache->tmctx, + &cache->hmctx); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + /* + * RBT-type cache DB has its own mechanism of cache cleaning and doesn't + * need the control of the generic cleaner. + */ + if (strcmp(db_type, "rbt") == 0) { + result = cache_cleaner_init(cache, NULL, NULL, &cache->cleaner); + } else { + result = cache_cleaner_init(cache, taskmgr, timermgr, + &cache->cleaner); + } + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + result = dns_db_setcachestats(cache->db, cache->stats); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + *cachep = cache; + return (ISC_R_SUCCESS); + +cleanup: + cache_free(cache); + return (result); +} + void dns_cache_attach(dns_cache_t *cache, dns_cache_t **targetp) { REQUIRE(VALID_CACHE(cache)); @@ -859,7 +870,7 @@ water(void *arg, int mark) { if (overmem != cache->cleaner.overmem) { dns_db_overmem(cache->db, overmem); cache->cleaner.overmem = overmem; - isc_mem_waterack(cache->mctx, mark); + isc_mem_waterack(cache->tmctx, mark); } if (cache->cleaner.overmem_event != NULL) { @@ -870,10 +881,19 @@ water(void *arg, int mark) { UNLOCK(&cache->cleaner.lock); } +static void +updatewater(dns_cache_t *cache) { + size_t hi = cache->size - (cache->size >> 3); /* ~ 7/8ths. */ + size_t lo = cache->size - (cache->size >> 2); /* ~ 3/4ths. */ + if (cache->size == 0U || hi == 0U || lo == 0U) { + isc_mem_clearwater(cache->tmctx); + } else { + isc_mem_setwater(cache->tmctx, water, cache, hi, lo); + } +} + void dns_cache_setcachesize(dns_cache_t *cache, size_t size) { - size_t hiwater, lowater; - REQUIRE(VALID_CACHE(cache)); /* @@ -886,30 +906,8 @@ dns_cache_setcachesize(dns_cache_t *cache, size_t size) { LOCK(&cache->lock); cache->size = size; + updatewater(cache); UNLOCK(&cache->lock); - - hiwater = size - (size >> 3); /* Approximately 7/8ths. */ - lowater = size - (size >> 2); /* Approximately 3/4ths. */ - - /* - * If the cache was overmem and cleaning, but now with the new limits - * it is no longer in an overmem condition, then the next - * isc_mem_put for cache memory will do the right thing and trigger - * water(). - */ - - if (size == 0U || hiwater == 0U || lowater == 0U) { - /* - * Disable cache memory limiting. - */ - isc_mem_clearwater(cache->mctx); - } else { - /* - * Establish new cache memory limits (either for the first - * time, or replacing other limits). - */ - isc_mem_setwater(cache->mctx, water, cache, hiwater, lowater); - } } size_t @@ -1004,8 +1002,10 @@ dns_cache_flush(dns_cache_t *cache) { dns_db_t *db = NULL, *olddb; dns_dbiterator_t *dbiterator = NULL, *olddbiterator = NULL; isc_result_t result; + isc_mem_t *tmctx = NULL, *oldtmctx; + isc_mem_t *hmctx = NULL, *oldhmctx; - result = cache_create_db(cache, &db); + result = cache_create_db(cache, &db, &tmctx, &hmctx); if (result != ISC_R_SUCCESS) { return (result); } @@ -1013,11 +1013,14 @@ dns_cache_flush(dns_cache_t *cache) { result = dns_db_createiterator(db, false, &dbiterator); if (result != ISC_R_SUCCESS) { dns_db_detach(&db); + isc_mem_detach(&tmctx); + isc_mem_detach(&hmctx); return (result); } LOCK(&cache->lock); LOCK(&cache->cleaner.lock); + isc_mem_clearwater(cache->tmctx); if (cache->cleaner.state == cleaner_s_idle) { olddbiterator = cache->cleaner.iterator; cache->cleaner.iterator = dbiterator; @@ -1028,6 +1031,11 @@ dns_cache_flush(dns_cache_t *cache) { } cache->cleaner.replaceiterator = true; } + oldhmctx = cache->hmctx; + cache->hmctx = hmctx; + oldtmctx = cache->tmctx; + cache->tmctx = tmctx; + updatewater(cache); olddb = cache->db; cache->db = db; dns_db_setcachestats(cache->db, cache->stats); @@ -1041,6 +1049,8 @@ dns_cache_flush(dns_cache_t *cache) { dns_dbiterator_destroy(&olddbiterator); } dns_db_detach(&olddb); + isc_mem_detach(&oldhmctx); + isc_mem_detach(&oldtmctx); return (ISC_R_SUCCESS); } @@ -1302,12 +1312,12 @@ dns_cache_dumpstats(dns_cache_t *cache, FILE *fp) { fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)dns_db_hashsize(cache->db), "cache database hash buckets"); - fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->mctx), + fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->tmctx), "cache tree memory total"); - fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->mctx), + fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_inuse(cache->tmctx), "cache tree memory in use"); fprintf(fp, "%20" PRIu64 " %s\n", - (uint64_t)isc_mem_maxinuse(cache->mctx), + (uint64_t)isc_mem_maxinuse(cache->tmctx), "cache tree highest memory in use"); fprintf(fp, "%20" PRIu64 " %s\n", (uint64_t)isc_mem_total(cache->hmctx), @@ -1372,9 +1382,9 @@ dns_cache_renderxml(dns_cache_t *cache, void *writer0) { dns_db_nodecount(cache->db, dns_dbtree_nsec), writer)); TRY0(renderstat("CacheBuckets", dns_db_hashsize(cache->db), writer)); - TRY0(renderstat("TreeMemTotal", isc_mem_total(cache->mctx), writer)); - TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->mctx), writer)); - TRY0(renderstat("TreeMemMax", isc_mem_maxinuse(cache->mctx), writer)); + TRY0(renderstat("TreeMemTotal", isc_mem_total(cache->tmctx), writer)); + TRY0(renderstat("TreeMemInUse", isc_mem_inuse(cache->tmctx), writer)); + TRY0(renderstat("TreeMemMax", isc_mem_maxinuse(cache->tmctx), writer)); TRY0(renderstat("HeapMemTotal", isc_mem_total(cache->hmctx), writer)); TRY0(renderstat("HeapMemInUse", isc_mem_inuse(cache->hmctx), writer)); @@ -1448,15 +1458,15 @@ dns_cache_renderjson(dns_cache_t *cache, void *cstats0) { CHECKMEM(obj); json_object_object_add(cstats, "CacheBuckets", obj); - obj = json_object_new_int64(isc_mem_total(cache->mctx)); + obj = json_object_new_int64(isc_mem_total(cache->tmctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemTotal", obj); - obj = json_object_new_int64(isc_mem_inuse(cache->mctx)); + obj = json_object_new_int64(isc_mem_inuse(cache->tmctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemInUse", obj); - obj = json_object_new_int64(isc_mem_maxinuse(cache->mctx)); + obj = json_object_new_int64(isc_mem_maxinuse(cache->tmctx)); CHECKMEM(obj); json_object_object_add(cstats, "TreeMemMax", obj); diff --git a/lib/dns/include/dns/cache.h b/lib/dns/include/dns/cache.h index 880c1ffd10..5b15a79c7c 100644 --- a/lib/dns/include/dns/cache.h +++ b/lib/dns/include/dns/cache.h @@ -56,7 +56,7 @@ ISC_LANG_BEGINDECLS *** Functions ***/ isc_result_t -dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, +dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, dns_rdataclass_t rdclass, const char *cachename, const char *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep); @@ -68,7 +68,7 @@ dns_cache_create(isc_mem_t *cmctx, isc_mem_t *hmctx, isc_taskmgr_t *taskmgr, * * Requires: * - *\li 'cmctx' and 'hmctx' are valid memory contexts. + *\li 'cmctx' are valid memory contexts. * *\li 'taskmgr' is a valid task manager (if 'db_type' is "rbt"). * diff --git a/tests/libtest/dns.c b/tests/libtest/dns.c index c01b1bb770..1432b34958 100644 --- a/tests/libtest/dns.c +++ b/tests/libtest/dns.c @@ -68,7 +68,7 @@ dns_test_makeview(const char *name, bool with_cache, dns_view_t **viewp) { } if (with_cache) { - result = dns_cache_create(mctx, mctx, taskmgr, timermgr, + result = dns_cache_create(mctx, taskmgr, timermgr, dns_rdataclass_in, "", "rbt", 0, NULL, &cache); if (result != ISC_R_SUCCESS) { From 13be6cd9912d5f6a70bbf0a3396630459e866711 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 27 Mar 2024 11:32:25 +1100 Subject: [PATCH 2/3] Pass a memory context in to dns_cache_create (cherry picked from commit 87e3b9dbf378c67a3e6822cedebbac0d57f0c64e) --- bin/named/server.c | 9 +-------- lib/dns/cache.c | 22 +++++++++++----------- lib/dns/include/dns/cache.h | 6 ++++-- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 97b4118d3b..fe65ddbae9 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4740,18 +4740,11 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * view but is not yet configured. If it is not the * view name but not a forward reference either, then it * is simply a named cache that is not shared. - * - * We use two separate memory contexts for the - * cache, for the main cache memory and the heap - * memory. */ - isc_mem_create(&cmctx); - isc_mem_setname(cmctx, "cache"); - CHECK(dns_cache_create(cmctx, named_g_taskmgr, + CHECK(dns_cache_create(mctx, named_g_taskmgr, named_g_timermgr, view->rdclass, cachename, "rbt", 0, NULL, &cache)); - isc_mem_detach(&cmctx); } nsc = isc_mem_get(mctx, sizeof(*nsc)); nsc->cache = NULL; diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 40732e9180..cb7f35a95f 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -127,7 +127,7 @@ struct dns_cache { /* Unlocked. */ unsigned int magic; isc_mutex_t lock; - isc_mem_t *mctx; /* Main cache memory */ + isc_mem_t *mctx; /* Memory context for the dns_cache object */ isc_mem_t *hmctx; /* Heap memory */ isc_mem_t *tmctx; /* Tree memory */ isc_taskmgr_t *taskmgr; @@ -331,7 +331,7 @@ cache_free(dns_cache_t *cache) { } isc_result_t -dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, +dns_cache_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, dns_rdataclass_t rdclass, const char *cachename, const char *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep) { @@ -341,31 +341,31 @@ dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, REQUIRE(cachep != NULL); REQUIRE(*cachep == NULL); - REQUIRE(cmctx != NULL); + REQUIRE(mctx != NULL); REQUIRE(taskmgr != NULL || strcmp(db_type, "rbt") != 0); REQUIRE(cachename != NULL); - cache = isc_mem_get(cmctx, sizeof(*cache)); + cache = isc_mem_get(mctx, sizeof(*cache)); *cache = (dns_cache_t){ - .db_type = isc_mem_strdup(cmctx, db_type), + .db_type = isc_mem_strdup(mctx, db_type), .rdclass = rdclass, .db_argc = db_argc, .name = cachename == NULL ? NULL - : isc_mem_strdup(cmctx, cachename), + : isc_mem_strdup(mctx, cachename), .magic = CACHE_MAGIC, }; - isc_mem_attach(cmctx, &cache->mctx); + isc_mutex_init(&cache->lock); + isc_mem_attach(mctx, &cache->mctx); if (taskmgr != NULL) { isc_taskmgr_attach(taskmgr, &cache->taskmgr); } - isc_mutex_init(&cache->lock); isc_refcount_init(&cache->references, 1); isc_refcount_init(&cache->live_tasks, 1); - result = isc_stats_create(cmctx, &cache->stats, + result = isc_stats_create(mctx, &cache->stats, dns_cachestatscounter_max); if (result != ISC_R_SUCCESS) { goto cleanup; @@ -382,7 +382,7 @@ dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, } if (cache->db_argc != 0) { - cache->db_argv = isc_mem_get(cmctx, + cache->db_argv = isc_mem_get(mctx, cache->db_argc * sizeof(char *)); for (i = 0; i < cache->db_argc; i++) { @@ -390,7 +390,7 @@ dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, } for (i = extra; i < cache->db_argc; i++) { - cache->db_argv[i] = isc_mem_strdup(cmctx, + cache->db_argv[i] = isc_mem_strdup(mctx, db_argv[i - extra]); } } diff --git a/lib/dns/include/dns/cache.h b/lib/dns/include/dns/cache.h index 5b15a79c7c..e5c6e49be9 100644 --- a/lib/dns/include/dns/cache.h +++ b/lib/dns/include/dns/cache.h @@ -56,7 +56,7 @@ ISC_LANG_BEGINDECLS *** Functions ***/ isc_result_t -dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, +dns_cache_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, dns_rdataclass_t rdclass, const char *cachename, const char *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep); @@ -68,7 +68,7 @@ dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, * * Requires: * - *\li 'cmctx' are valid memory contexts. + *\li 'mctx' is a valid memory context. * *\li 'taskmgr' is a valid task manager (if 'db_type' is "rbt"). * @@ -77,6 +77,8 @@ dns_cache_create(isc_mem_t *cmctx, isc_taskmgr_t *taskmgr, * periodic cleaning of the cache will take place. * *\li 'cachename' is a valid string. This must not be NULL. + + *\li 'mctx' is a valid memory context. * *\li 'cachep' is a valid pointer, and *cachep == NULL * From 455c26264931d606f2a47cb1db770d7540bf3b20 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 16 Feb 2024 11:53:23 +1100 Subject: [PATCH 3/3] Add CHANGES entry for [GL #2744] (cherry picked from commit 5be6ceebc47e8f87aaa215fedb8618821dbc4e79) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index ac9529a04a..19cbb8046c 100644 --- a/CHANGES +++ b/CHANGES @@ -9,6 +9,9 @@ 6393. [func] Deal with uv_tcp_close_reset() error return codes more gracefully. [GL #4708] +6392. [bug] Use a completely new memory context when flushing the + cache. [GL #2744] + 6391. [bug] TCP client statistics could sometimes fail to decrease when accepting client connection fails. [GL #4742]