When a function returns void, it can be used as an argument to return in
function returning also void, e.g.:
void in(void) {
return;
}
void out(void) {
return (in());
}
while this is legal, it should be rewritten as:
void out(void) {
in();
return;
}
The semantic patch just find the occurrences, and they need to be fixed
by hand.
Since the introduction of durations, all ttlval configuration types
are replaced with durations. Duration is an ISO 8601 duration, a
TTL-style value, or a number. These two references were missed and
are now also replaced.
This commit makes some minor changes to the trust anchor code:
1. Replace the undescriptive n1, n2 and n3 identifiers with slightly
better rdata1, rdata2, and rdata3.
2. Fix an occurrence where in the error log message a static number
32 was printed, rather than the rdata3 length.
3. Add a default case to the switch statement checking DS digest
algorithms to catch unknown algorithms.
Previously, the fetchlimit tested the recursive-clients soft limit
that's defined as 90% of the hard limit (the actual configured value).
This worked previously because the reaping of the oldest recursive
client was put on the same event queue as the current TCP client, thus
the cleaning has happened before the new TCP client established a new
connection.
With the change in BIND 9.14 that added a multiple event queues the
cleaning of the oldests clients is no longer synchronous and could
happen stochastically making the soft limit testing fail often. The
situation became even worse with the new networking manager, thus we
change the system test to fail only if the hard limit bound is not
honored.
Changing the accounting of the already reaped TCP clients so the soft
limit testing is possible again is out of the scope for this change.
These two tests were failing basically because in order for prefetching to
happen, the TTL for a given DNS record must be greater than or equal to
the prefetch config value + 9.
The previous TTL for both records was 10, while prefetch value in
configuration was 3, thus making only records with TTL >= 12 elligible
for prefetching.
TTL value for both records was adjusted to the value 13, and prefetch
value was set to 4 (inc by 1), so records with TTL (4 + 9) >= 13 are
elligible for prefetching.
Adjusting prefetch value to 4 gives the test 1 second more to avoid time
problems when sharing resources on a heavy loaded PC.
Also prefetch value in settings is now read by the script and used
by it to corrrectly calculate the amount of time needed to delay before
sending a request to trigger prefetch, adding a bit of flexibility to
fine tune the test in the future.
The previous test had two problems:
1. It wasn't written specifically for testing what it was supposed to:
prefetch disabled.
2. It could fail in some circunstances if the computer's load is too
high, due to sleeps not taking parallel tests and cpu load into account.
The new test is testing prefetch disabled as follows:
1. It asks for a txt record for a given domain and takes note of the
record's TTL (which is 10).
2. It sleeps for (TTL - 5) = 5 seconds, having a window of 5 seconds to
issue new queries before the record expires from cache.
3. Three(3) queries are executed in a row, with a interval of 1 second
between them, and for each query we verify that the TTL in response is
less than the previous one, thus ensuring that prefetch is disabled (if
it were enabled this record would have been refreshed already and TTL
would be >= the first TTL).
Having a window of 5 seconds to perform 3 queries with a interval of 1
second between them gives the test a reasonable amount of time
to not suffer from a machine with heavy load.
For BIND 9.16+, TLS aware compiler is required, and using
ISC_THREAD_LOCAL is preferred way of using Thread Local Storage. The
isc_thread_key API is no longer used anywhere and hence was removed from
BIND 9.
Previously, the irs_context API used isc_thread_key API for TLS, which is
fairly complicated and requires initialization of memory contexts, etc.
This part of code was refactored to use a ISC_THREAD_LOCAL pointer which
greatly simplifies the whole code related to storing TLS variables.
Previously, the dns_geoip API used isc_thread_key API for TLS, which is
fairly complicated and requires initialization of memory contexts, etc.
This part of code was refactored to use a ISC_THREAD_LOCAL pointer which
greatly simplifies the whole code related to storing TLS variables, and
creating the local memory context was moved to named and stored in the
named_g_geoip global context.
Previously, the dns_dt API used isc_thread_key API for TLS, which is
fairly complicated and requires initialization of memory contexts, etc.
This part of code was refactored to use a ISC_THREAD_LOCAL pointer which
greatly simplifies the whole code related to storing TLS variables.
Previously, the dns_name API used isc_thread_key API for TLS, which is
fairly complicated and requires initialization of memory contexts, etc.
This part of code was refactored to use a ISC_THREAD_LOCAL pointer which
greatly simplifies the whole code related to storing TLS variables.
The new ISC_THREAD_LOCAL macro unifies usage of platform dependent
Thread Local Storage definition thread_local vs __thread vs
__declspec(thread) to a single macro.
The commit also unifies the required level of support for TLS as for
some parts of the code it was mandatory and for some parts of the code
it wasn't.
FCTX_ATTR_SHUTTINGDOWN needs to be set and tested while holding the node
lock but the rest of the attributes don't as they are task locked. Making
fctx->attributes atomic allows both behaviours without races.
Disabling ASAN memory leak detection for a build job is pointless
because ASAN is only used in test jobs. (Also, memory leak detection
should not be disabled globally - explicit suppressions should be used
in case of issues with external code.)
xmlInitThreads() and xmlCleanupThreads() are called from within
named_statschannels_configure() and named_statschannels_shutdown(),
respectively. Both of these functions are executed by worker threads,
not the main named thread. This causes ASAN to report memory leaks like
the following one upon shutdown (as long as named is asked to produce
any XML output over its configured statistics channels during its
lifetime):
Direct leak of 968 byte(s) in 1 object(s) allocated from:
#0 0x7f677c249cd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
#1 0x7f677bc1838f in xmlGetGlobalState (/usr/lib/libxml2.so.2+0xa838f)
The data mentioned in the above report is a libxml2 state structure
stored as thread-specific data. Such chunks of memory are automatically
released (by a destructor passed to pthread_key_create() by libxml2)
whenever a thread that allocated a given chunk exits. However, if
xmlCleanupThreads() is called by a given thread before it exits, the
destructor will not be invoked (due to xmlCleanupThreads() calling
pthread_key_delete()) and ASAN will report a memory leak. Thus,
xmlInitThreads() and xmlCleanupThreads() must not be called from worker
threads. Since xmlInitThreads() must be called on Windows in order for
libxml2 to work at all, move xmlInitThreads() and xmlCleanupThreads()
calls to the main named thread (which does not produce any XML output
itself) in order to prevent the memory leak from being reported by ASAN.
Loaded GeoIP2 databases are only released when named is shut down, but
not during server reconfiguration. This causes memory to be leaked
every time "rndc reconfig" or "rndc reload" is used, as long as any
GeoIP2 database is in use. Fix by releasing any loaded GeoIP2 databases
before reloading them. Do not call dns_geoip_shutdown() until server
shutdown as that function releases the memory context used for caching
GeoIP2 lookup results.
This prevents races on fctx->client whenever a new fetch joins a existing
fetch (by calling fctx_join) as it is now invariant for the active life of
fctx.
Some semantic patches are meant to be run just once, as they work on
functions with changed prototypes. We keep them for reference, but
disabled them from the CI to save time.
