Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Remove isc_thread_key API in favor of ISC_THREAD_LOCAL variables
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.
Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Refactor the irs_context API to use ISC_THREAD_LOCAL
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.
Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Refactor the dns_geoip API to use ISC_THREAD_LOCAL
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.
Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Refactor the dns_dt API to use ISC_THREAD_LOCAL
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.
Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Refactor the dns_name API to use ISC_THREAD_LOCAL
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.
Ondřej Surý [Mon, 2 Dec 2019 10:42:50 +0000 (11:42 +0100)]
Add and use ISC_THREAD_LOCAL macro
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.
Mark Andrews [Mon, 2 Dec 2019 04:11:50 +0000 (15:11 +1100)]
Make fctx->attributes atomic.
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.
Michał Kępień [Mon, 2 Dec 2019 15:03:23 +0000 (16:03 +0100)]
Do not define ASAN_OPTIONS at build time
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.)
Michał Kępień [Mon, 2 Dec 2019 15:03:23 +0000 (16:03 +0100)]
Move xmlInitThreads()/xmlCleanupThreads() calls
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.
Michał Kępień [Mon, 2 Dec 2019 14:15:06 +0000 (15:15 +0100)]
Fix GeoIP2 memory leak upon reconfiguration
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.
Mark Andrews [Thu, 28 Nov 2019 22:59:03 +0000 (09:59 +1100)]
Assign fctx->client when fctx is created rather when the join happens.
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.
Ondřej Surý [Fri, 29 Nov 2019 13:03:29 +0000 (14:03 +0100)]
Disable no longer useful semantic patches
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.
Michał Kępień [Fri, 29 Nov 2019 08:06:41 +0000 (09:06 +0100)]
Fix logging long named command lines
The saved_command_line buffer in bin/named/main.c is 8192 bytes long.
The size of libisc's internal logging buffer (defined by the value of
the LOG_BUFFER_SIZE constant in lib/isc/log.c) is also 8192 bytes.
Since the buffer containing the ellipsis is passed as the last argument
to isc_log_write() and the buffer containing the potentially trimmed
named command line (saved_command_line) is passed as the second argument
in the same isc_log_write() call, it may happen that saved_command_line
will exhaust all available space in libisc's internal logging buffer, in
which case the ellipsis will be elided from the output.
Make saved_command_line 4096 bytes long as that value is arguably also
large enough for any reasonable use case and at the same time it ensures
ellipsis will always be printed for excessively long named command
lines.
Michał Kępień [Fri, 29 Nov 2019 07:47:33 +0000 (08:47 +0100)]
Improve portability of the "runtime" system test
The "runtime" system test currently fails on Windows because it waits
for named to log a message indicating successful startup ("running"),
but that never happens since named on Windows fails to open the
configuration file as its path includes control characters.
Instead of putting control characters in directory names, put them in
the value of the -D command line switch passed to named, which is used
for identifying an instance of named in a process listing and whose
value is completely ignored by named, but still logged.
While a similar check using special characters appears to be working
fine on Windows for the time being, modify it in the same way to avoid
potential future problems on other platforms and make the test cleaner.
Ondřej Surý [Sat, 16 Nov 2019 05:36:23 +0000 (13:36 +0800)]
Make runtime/tests.sh shellcheck and set -e clean
This mostly comprises of:
* using $(...) instead of `...`
* changing the directories in subshell and not ignoring `cd` return code
* handling every error gracefully instead of ignoring the return code
Mark Andrews [Mon, 18 Nov 2019 09:46:58 +0000 (20:46 +1100)]
dns_master_indent and dns_master_indentstr must not be global
The indentation for dumping the master zone was driven by two
global variables dns_master_indent and dns_master_indentstr. In
threaded mode, this becomes prone to data access races, so this commit
converts the global variables into a local per-context tuple that
consist of count and string.
Ondřej Surý [Fri, 22 Nov 2019 10:39:57 +0000 (11:39 +0100)]
Request exclusive access when crashing via fatal()
When loading the configuration fails, there might be already other tasks
running and calling OpenSSL library functions. The OpenSSL on_exit
handler is called when exiting the main process and there's a timing
race between the on_exit function that destroys OpenSSL allocated
resources (threads, locks, ...) and other tasks accessing the very same
resources leading to a crash in the system threading library. Therefore,
the fatal() function needs to request exlusive access to the task
manager to finish the already running tasks and exit only when no other
tasks are running.
Ondřej Surý [Wed, 20 Nov 2019 19:43:35 +0000 (03:43 +0800)]
Tune the performance of oldsigs test
The oldsigs test was checking only for the validity of the A
a.oldsigs.example. resource record and associated DNSSEC signature while
the zone might not have been fully signed yet leading to validation
failures because of bogus signatures on the validation path.
This commit changes the test to test that all old signatures in the
oldsigs.example. zone were replaced and the zone is fully resigned
before running the main check.