2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-01 06:55:30 +00:00

Refactor the dns_resolver fetch context hash tables and locking

This is second in the series of fixing the usage of hashtables in the
dns_adb and the dns_resolver units.

Currently, the fetch buckets (used to hold the fetch context) and zone
buckets (used to hold per-domain counters) would never get cleaned from
the memory.  Combined with the fact that the hashtable now grows as
needed (instead of using hashtable as buckets), the memory usage in the
resolver can just grow and it never drops down.

In this commit, the usage of hashtables (hashmaps) has been completely
rewritten, so there are no "buckets" and all the matching conditions are
directly mapped into the hashtable key:

 1. For per-domain counter hashtable, this is simple as the lowercase
    domain name is used directly as a counter.

 2. For fetch context hashtable, this requires copying some extra flags
    back and forth in the key.

As we don't hold the "buckets" forever, the cleaning mechanism has been
rewritten as well:

 1. For per-domain counter hashtable, this is again much simpler, as we
    only need to check whether the usage counter is still zero under the
    lock and bail-out on cleaning if the counter is in use.

 2. For fetch context hashtable, this is more complicated as the fetch
    context cannot be reused after it has been finished.  The algorithm
    is different, the fetch context is always removed from the
    hashtable, but if we find the fetch context that has been marked
    as finished in the lookup function, we help with the cleaning from
    the hashtable and try again.

Couple of additional changes have been implemented in this refactoring
as those were needed for correct functionality and could not be split
into individual commits (or would not make sense as seperate commits):

 1. The dns_resolver_createfetch() has an option to create "unshared"
    fetch.  The "unshared" fetch will never get matched, so there's
    little point in storing the "unshared" fetch in the hashtable.
    Therefore the "unshared" fetches are now detached from the
    hashtable and live just on their own.

 2. Replace the custom reference counting with ISC_REFCOUNT_DECL/IMPL
    macros for better tracing.

 3. fctx_done_detach() is idempotent, it makes the "final" detach (the
    one matching the create function) only once.  But that also means
    that it has to be called before the detach that kept the fetch
    context alive in the callback.  A new macro fctx_done_unref() has
    been added to allow this code flow:

    fctx_done_unref(fctx, result);
    fctx_detach(&fctx);

    Doing this the other way around could cause fctx to get destroyed in
    the fctx_unref() first and fctx_done_detach() would cause UAF.

 4. The resume_qmin() and resume_dslookup() callbacks have been
    refactored for more readability and simpler code paths.  The
    validated() callback has also received some of the simplifications,
    but it should be refactored in the future as it is bit of spaghetti
    now.
This commit is contained in:
Ondřej Surý
2022-11-04 15:04:29 +01:00
committed by Ondřej Surý
parent fbc9d14149
commit 7e4e125e5e
2 changed files with 853 additions and 1061 deletions

View File

@@ -50,11 +50,16 @@
#include <isc/event.h>
#include <isc/lang.h>
#include <isc/refcount.h>
#include <isc/stats.h>
#include <dns/fixedname.h>
#include <dns/types.h>
#include <netinet/in.h>
#undef DNS_RESOLVER_TRACE
ISC_LANG_BEGINDECLS
/*%
@@ -253,11 +258,19 @@ dns_resolver_shutdown(dns_resolver_t *res);
*\li 'res' is a valid resolver.
*/
void
dns_resolver_attach(dns_resolver_t *source, dns_resolver_t **targetp);
void
dns_resolver_detach(dns_resolver_t **resp);
#if DNS_RESOLVER_TRACE
#define dns_resolver_ref(ptr) \
dns_resolver__ref(ptr, __func__, __FILE__, __LINE__)
#define dns_resolver_unref(ptr) \
dns_resolver__unref(ptr, __func__, __FILE__, __LINE__)
#define dns_resolver_attach(ptr, ptrp) \
dns_resolver__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
#define dns_resolver_detach(ptrp) \
dns_resolver__detach(ptrp, __func__, __FILE__, __LINE__)
ISC_REFCOUNT_TRACE_DECL(dns_resolver);
#else
ISC_REFCOUNT_DECL(dns_resolver);
#endif
isc_result_t
dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name,

File diff suppressed because it is too large Load Diff