]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
3053. [bug] Under a sustained high query load with a finite
authorEvan Hunt <each@isc.org>
Thu, 3 Mar 2011 04:43:02 +0000 (04:43 +0000)
committerEvan Hunt <each@isc.org>
Thu, 3 Mar 2011 04:43:02 +0000 (04:43 +0000)
max-cache-size, it was possible for cache memory
to be exhausted and not recovered. [RT #23371]

CHANGES
bin/named/server.c
lib/dns/cache.c
lib/dns/include/dns/cache.h
lib/dns/rbtdb.c
lib/dns/rbtdb.h
lib/isc/heap.c

diff --git a/CHANGES b/CHANGES
index b068f47128694bcdf46f0271c976c23aa1e11334..f14ebe4306299cee3f63c567d230f54056d661bd 100644 (file)
--- 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
index e050f50991b32b6e0e742178b8cb28d64e8635d8..056f656fa2eaf8dedce5f81e1a832c7e9e09abf3 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: server.c,v 1.599.8.4 2011/02/16 19:46:12 each Exp $ */
+/* $Id: server.c,v 1.599.8.5 2011/03/03 04:43:01 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);
index 6c971441b95936c9d79e5d31989486ce107c7637..cafab49ef538b271a525f240de78e257598d5eb1 100644 (file)
@@ -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.87.262.1 2011/03/03 04:43:01 each Exp $ */
 
 /*! \file */
 
@@ -40,6 +40,8 @@
 #include <dns/rdatasetiter.h>
 #include <dns/result.h>
 
+#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));
 }
 
 
index 1a59ea9ef2a647f73ced67eee36ad1fd9acaa254..00f67d4c264ea7932158f91a5627f4bee782ed62 100644 (file)
@@ -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.28.428.1 2011/03/03 04:43:02 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
index 9ade3e911e31b1d980ed21f2120ba42c032879a8..04cf1df46ac86fbbb4290fbac2fdf233ba84a743 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: rbtdb.c,v 1.310.8.2 2011/03/02 04:25:15 marka Exp $ */
+/* $Id: rbtdb.c,v 1.310.8.3 2011/03/03 04:43:01 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.
index b024d136e838ecf9232f9083042e18fe8b53515d..70143fcfee0255541b75c3999597aa0f9e152395 100644 (file)
@@ -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.18.814.1 2011/03/03 04:43:02 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 */
index 7efbd1a5e71669d5f4d4eb4ab552e3bf54053766..dfdf3324eaf76026573ad635dd091f7852b92ece 100644 (file)
@@ -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.39.150.1 2011/03/03 04:43:02 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;
 }