diff --git a/CHANGES b/CHANGES index 95947c0ca9..062279936e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +3053. [bug] Under a sustained high query load with a finite + max-cache-size, it was possible for cache memory + to be exhausted and not recovered. [RT #23371] + 3052. [test] Fixed last autosign test report. [RT #23256] 3051. [bug] NS records obsure DS records at the bottom of the diff --git a/bin/named/server.c b/bin/named/server.c index d5e2445964..dac5755583 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: server.c,v 1.604 2011/02/23 03:08:09 marka Exp $ */ +/* $Id: server.c,v 1.605 2011/03/03 04:42:25 each Exp $ */ /*! \file */ @@ -1587,7 +1587,7 @@ configure_view(dns_view_t *view, cfg_parser_t* parser, isc_uint32_t lame_ttl; dns_tsig_keyring_t *ring = NULL; dns_view_t *pview = NULL; /* Production view */ - isc_mem_t *cmctx; + isc_mem_t *cmctx = NULL, *hmctx = NULL; dns_dispatch_t *dispatch4 = NULL; dns_dispatch_t *dispatch6 = NULL; isc_boolean_t reused_cache = ISC_FALSE; @@ -1619,8 +1619,6 @@ configure_view(dns_view_t *view, cfg_parser_t* parser, REQUIRE(DNS_VIEW_VALID(view)); - cmctx = NULL; - if (config != NULL) (void)cfg_map_get(config, "options", &options); @@ -2103,13 +2101,21 @@ configure_view(dns_view_t *view, cfg_parser_t* parser, * 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. */ CHECK(isc_mem_create(0, 0, &cmctx)); isc_mem_setname(cmctx, "cache", NULL); - CHECK(dns_cache_create2(cmctx, ns_g_taskmgr, + CHECK(isc_mem_create(0, 0, &hmctx)); + isc_mem_setname(hmctx, "cache_heap", NULL); + CHECK(dns_cache_create3(cmctx, hmctx, ns_g_taskmgr, ns_g_timermgr, view->rdclass, cachename, "rbt", 0, NULL, &cache)); + isc_mem_detach(&cmctx); + isc_mem_detach(&hmctx); } nsc = isc_mem_get(mctx, sizeof(*nsc)); if (nsc == NULL) { @@ -2947,6 +2953,8 @@ configure_view(dns_view_t *view, cfg_parser_t* parser, dns_order_detach(&order); if (cmctx != NULL) isc_mem_detach(&cmctx); + if (hmctx != NULL) + isc_mem_detach(&hmctx); if (cache != NULL) dns_cache_detach(&cache); diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 6c971441b9..b230f5d600 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: cache.c,v 1.87 2009/11/12 23:43:02 each Exp $ */ +/* $Id: cache.c,v 1.88 2011/03/03 04:42:25 each Exp $ */ /*! \file */ @@ -40,6 +40,8 @@ #include #include +#include "rbtdb.h" + #define CACHE_MAGIC ISC_MAGIC('$', '$', '$', '$') #define VALID_CACHE(cache) ISC_MAGIC_VALID(cache, CACHE_MAGIC) @@ -121,7 +123,8 @@ struct dns_cache { unsigned int magic; isc_mutex_t lock; isc_mutex_t filelock; - isc_mem_t *mctx; + isc_mem_t *mctx; /* Main cache memory */ + isc_mem_t *hmctx; /* Heap memory */ char *name; /* Locked by 'lock'. */ @@ -168,41 +171,54 @@ cache_create_db(dns_cache_t *cache, dns_db_t **db) { } isc_result_t -dns_cache_create(isc_mem_t *mctx, 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 *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep) { - return (dns_cache_create2(mctx, taskmgr, timermgr, rdclass, "", + return (dns_cache_create3(cmctx, cmctx, taskmgr, timermgr, rdclass, "", db_type, db_argc, db_argv, cachep)); } isc_result_t -dns_cache_create2(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, +dns_cache_create2(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) +{ + return (dns_cache_create3(cmctx, cmctx, taskmgr, timermgr, rdclass, + cachename, db_type, db_argc, db_argv, + cachep)); +} + +isc_result_t +dns_cache_create3(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; + int i, extra = 0; isc_task_t *dbtask; REQUIRE(cachep != NULL); REQUIRE(*cachep == NULL); - REQUIRE(mctx != NULL); + REQUIRE(cmctx != NULL); + REQUIRE(hmctx != NULL); REQUIRE(cachename != NULL); - cache = isc_mem_get(mctx, sizeof(*cache)); + cache = isc_mem_get(cmctx, sizeof(*cache)); if (cache == NULL) return (ISC_R_NOMEMORY); - cache->mctx = NULL; - isc_mem_attach(mctx, &cache->mctx); + cache->mctx = cache->hmctx = NULL; + isc_mem_attach(cmctx, &cache->mctx); + isc_mem_attach(hmctx, &cache->hmctx); cache->name = NULL; if (cachename != NULL) { - cache->name = isc_mem_strdup(mctx, cachename); + cache->name = isc_mem_strdup(cmctx, cachename); if (cache->name == NULL) { result = ISC_R_NOMEMORY; goto cleanup_mem; @@ -221,26 +237,38 @@ dns_cache_create2(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, cache->live_tasks = 0; cache->rdclass = rdclass; - cache->db_type = isc_mem_strdup(mctx, db_type); + cache->db_type = isc_mem_strdup(cmctx, db_type); if (cache->db_type == NULL) { result = ISC_R_NOMEMORY; goto cleanup_filelock; } - cache->db_argc = db_argc; - if (cache->db_argc == 0) - cache->db_argv = NULL; - else { - cache->db_argv = isc_mem_get(mctx, + /* + * 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 *)); if (cache->db_argv == NULL) { result = ISC_R_NOMEMORY; goto cleanup_dbtype; } + for (i = 0; i < cache->db_argc; i++) cache->db_argv[i] = NULL; - for (i = 0; i < cache->db_argc; i++) { - cache->db_argv[i] = isc_mem_strdup(mctx, db_argv[i]); + + 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]); if (cache->db_argv[i] == NULL) { result = ISC_R_NOMEMORY; goto cleanup_dbargv; @@ -248,6 +276,9 @@ dns_cache_create2(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, } } + /* + * Create the database + */ cache->db = NULL; result = cache_create_db(cache, &cache->db); if (result != ISC_R_SUCCESS) @@ -284,29 +315,28 @@ dns_cache_create2(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, cleanup_db: dns_db_detach(&cache->db); cleanup_dbargv: - for (i = 0; i < cache->db_argc; i++) + for (i = extra; i < cache->db_argc; i++) if (cache->db_argv[i] != NULL) - isc_mem_free(mctx, cache->db_argv[i]); + isc_mem_free(cmctx, cache->db_argv[i]); if (cache->db_argv != NULL) - isc_mem_put(mctx, cache->db_argv, + isc_mem_put(cmctx, cache->db_argv, cache->db_argc * sizeof(char *)); cleanup_dbtype: - isc_mem_free(mctx, cache->db_type); + isc_mem_free(cmctx, cache->db_type); cleanup_filelock: DESTROYLOCK(&cache->filelock); cleanup_lock: DESTROYLOCK(&cache->lock); cleanup_mem: if (cache->name != NULL) - isc_mem_free(mctx, cache->name); - isc_mem_put(mctx, cache, sizeof(*cache)); - isc_mem_detach(&mctx); + isc_mem_free(cmctx, cache->name); + isc_mem_detach(&cache->hmctx); + isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); return (result); } static void cache_free(dns_cache_t *cache) { - isc_mem_t *mctx; int i; REQUIRE(VALID_CACHE(cache)); @@ -337,7 +367,14 @@ cache_free(dns_cache_t *cache) { dns_db_detach(&cache->db); if (cache->db_argv != NULL) { - for (i = 0; i < cache->db_argc; i++) + /* + * We don't free db_argv[0] in "rbt" cache databases + * as it's a pointer to hmctx + */ + int extra = 0; + if (strcmp(cache->db_type, "rbt") == 0) + extra = 1; + for (i = extra; i < cache->db_argc; i++) if (cache->db_argv[i] != NULL) isc_mem_free(cache->mctx, cache->db_argv[i]); isc_mem_put(cache->mctx, cache->db_argv, @@ -352,10 +389,10 @@ cache_free(dns_cache_t *cache) { DESTROYLOCK(&cache->lock); DESTROYLOCK(&cache->filelock); + cache->magic = 0; - mctx = cache->mctx; - isc_mem_put(cache->mctx, cache, sizeof(*cache)); - isc_mem_detach(&mctx); + isc_mem_detach(&cache->hmctx); + isc_mem_putanddetach(&cache->mctx, cache, sizeof(*cache)); } diff --git a/lib/dns/include/dns/cache.h b/lib/dns/include/dns/cache.h index 1a59ea9ef2..eec4472288 100644 --- a/lib/dns/include/dns/cache.h +++ b/lib/dns/include/dns/cache.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: cache.h,v 1.28 2009/01/09 23:47:46 tbox Exp $ */ +/* $Id: cache.h,v 1.29 2011/03/03 04:42:25 each Exp $ */ #ifndef DNS_CACHE_H #define DNS_CACHE_H 1 @@ -61,23 +61,36 @@ ISC_LANG_BEGINDECLS ***/ isc_result_t -dns_cache_create(isc_mem_t *mctx, 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 *db_type, unsigned int db_argc, char **db_argv, dns_cache_t **cachep); isc_result_t -dns_cache_create2(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, +dns_cache_create2(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 +dns_cache_create3(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); /*%< - * Create a new DNS cache. dns_cache_create2() will create a named cache. - * dns_cache_create() is a backward compatible version that internally specifies - * an empty name. + * Create a new DNS cache. + * + * dns_cache_create2() will create a named cache. + * + * dns_cache_create3() will create a named cache using two separate memory + * contexts, one for cache data which can be cleaned and a separate one for + * memory allocated for the heap (which can grow without an upper limit and + * has no mechanism for shrinking). + * + * dns_cache_create() is a backward compatible version that internally + * specifies an empty cache name and a single memory context. * * Requires: * - *\li 'mctx' is a valid memory context + *\li 'cmctx' (and 'hmctx' if applicable) is a valid memory context. * *\li 'taskmgr' is a valid task manager and 'timermgr' is a valid timer * manager, or both are NULL. If NULL, no periodic cleaning of the diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index cd149c170f..c3fe019714 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: rbtdb.c,v 1.312 2011/03/02 04:20:34 marka Exp $ */ +/* $Id: rbtdb.c,v 1.313 2011/03/03 04:42:25 each Exp $ */ /*! \file */ @@ -433,8 +433,12 @@ typedef struct { rbtnodelist_t *deadnodes; /* - * Heaps. Each of these is used for TTL based expiry. + * Heaps. These are used for TTL based expiry in a cache, + * or for zone resigning in a zone DB. hmctx is the memory + * context to use for the heap (which differs from the main + * database memory context in the case of a cache). */ + isc_mem_t * hmctx; isc_heap_t **heaps; /* Locked by tree_lock. */ @@ -950,9 +954,8 @@ free_rbtdb(dns_rbtdb_t *rbtdb, isc_boolean_t log, isc_event_t *event) { if (rbtdb->heaps != NULL) { for (i = 0; i < rbtdb->node_lock_count; i++) isc_heap_destroy(&rbtdb->heaps[i]); - isc_mem_put(rbtdb->common.mctx, rbtdb->heaps, - rbtdb->node_lock_count * - sizeof(isc_heap_t *)); + isc_mem_put(rbtdb->hmctx, rbtdb->heaps, + rbtdb->node_lock_count * sizeof(isc_heap_t *)); } if (rbtdb->rrsetstats != NULL) @@ -974,6 +977,7 @@ free_rbtdb(dns_rbtdb_t *rbtdb, isc_boolean_t log, isc_event_t *event) { rbtdb->common.magic = 0; rbtdb->common.impmagic = 0; ondest = rbtdb->common.ondest; + isc_mem_detach(&rbtdb->hmctx); isc_mem_putanddetach(&rbtdb->common.mctx, rbtdb, sizeof(*rbtdb)); isc_ondestroy_notify(&ondest, rbtdb); } @@ -7459,16 +7463,21 @@ dns_rbtdb_create int i; dns_name_t name; isc_boolean_t (*sooner)(void *, void *); + isc_mem_t *hmctx = mctx; /* Keep the compiler happy. */ - UNUSED(argc); - UNUSED(argv); UNUSED(driverarg); rbtdb = isc_mem_get(mctx, sizeof(*rbtdb)); if (rbtdb == NULL) return (ISC_R_NOMEMORY); + /* + * If argv[0] exists, it points to a memory context to use for heap + */ + if (argc != 0) + hmctx = (isc_mem_t *) argv[0]; + memset(rbtdb, '\0', sizeof(*rbtdb)); dns_name_init(&rbtdb->common.origin, NULL); rbtdb->common.attributes = 0; @@ -7533,7 +7542,7 @@ dns_rbtdb_create /* * Create the heaps. */ - rbtdb->heaps = isc_mem_get(mctx, rbtdb->node_lock_count * + rbtdb->heaps = isc_mem_get(hmctx, rbtdb->node_lock_count * sizeof(isc_heap_t *)); if (rbtdb->heaps == NULL) { result = ISC_R_NOMEMORY; @@ -7543,7 +7552,7 @@ dns_rbtdb_create rbtdb->heaps[i] = NULL; sooner = IS_CACHE(rbtdb) ? ttl_sooner : resign_sooner; for (i = 0; i < (int)rbtdb->node_lock_count; i++) { - result = isc_heap_create(mctx, sooner, set_index, 0, + result = isc_heap_create(hmctx, sooner, set_index, 0, &rbtdb->heaps[i]); if (result != ISC_R_SUCCESS) goto cleanup_heaps; @@ -7587,6 +7596,7 @@ dns_rbtdb_create * mctx won't disappear out from under us. */ isc_mem_attach(mctx, &rbtdb->common.mctx); + isc_mem_attach(hmctx, &rbtdb->hmctx); /* * Must be initialized before free_rbtdb() is called. diff --git a/lib/dns/rbtdb.h b/lib/dns/rbtdb.h index b024d136e8..ee9fb7d261 100644 --- a/lib/dns/rbtdb.h +++ b/lib/dns/rbtdb.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: rbtdb.h,v 1.18 2007/06/19 23:47:16 tbox Exp $ */ +/* $Id: rbtdb.h,v 1.19 2011/03/03 04:42:25 each Exp $ */ #ifndef DNS_RBTDB_H #define DNS_RBTDB_H 1 @@ -39,6 +39,19 @@ dns_rbtdb_create(isc_mem_t *mctx, dns_name_t *base, dns_dbtype_t type, dns_rdataclass_t rdclass, unsigned int argc, char *argv[], void *driverarg, dns_db_t **dbp); +/*%< + * Create a new database of type "rbt" (or "rbt64"). Called via + * dns_db_create(); see documentation for that function for more details. + * + * If argv[0] is set, it points to a valid memory context to be used for + * allocation of heap memory. Generally this is used for cache databases + * only. + * + * Requires: + * + * \li argc == 0 or argv[0] is a valid memory context. + */ + ISC_LANG_ENDDECLS #endif /* DNS_RBTDB_H */ diff --git a/lib/isc/heap.c b/lib/isc/heap.c index 7efbd1a5e7..a976ce0f9b 100644 --- a/lib/isc/heap.c +++ b/lib/isc/heap.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: heap.c,v 1.39 2010/02/04 23:49:13 tbox Exp $ */ +/* $Id: heap.c,v 1.40 2011/03/03 04:42:25 each Exp $ */ /*! \file * Heap implementation of priority queues adapted from the following: @@ -86,8 +86,9 @@ isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare, if (heap == NULL) return (ISC_R_NOMEMORY); heap->magic = HEAP_MAGIC; - heap->mctx = mctx; heap->size = 0; + heap->mctx = NULL; + isc_mem_attach(mctx, &heap->mctx); if (size_increment == 0) heap->size_increment = SIZE_INCREMENT; else @@ -114,7 +115,7 @@ isc_heap_destroy(isc_heap_t **heapp) { isc_mem_put(heap->mctx, heap->array, heap->size * sizeof(void *)); heap->magic = 0; - isc_mem_put(heap->mctx, heap, sizeof(*heap)); + isc_mem_putanddetach(&heap->mctx, heap, sizeof(*heap)); *heapp = NULL; }