The rwlock introduced to protect the .logconfig member of isc_log_t
structure caused a significant performance drop because of the rwlock
contention. It was also found, that the debug_level member of said
structure was not protected from concurrent read/writes.
The .dynamic and .highest_level members of isc_logconfig_t structure
were actually just cached values pulled from the assigned channels.
We introduced an even higher cache level for .dynamic and .highest_level
members directly into the isc_log_t structure, so we don't have to
access the .logconfig member in the isc_log_wouldlog() function.
In isc_log_woudlog() the .logconfig member of isc_log_t structure was
accessed unlocked on the merit that there could be just a race when
.logconfig would be NULL, so the message would not be logged. This
turned not to be true, as there's also data race deeper. The accessed
isc_logconfig_t object could be in the middle of destruction, so the
pointer would be still non-NULL, but the structure members could point
to a chunk of memory no longer belonging to the object. Since we are
only accessing integer types (the log level), this would never lead to
a crash, it leads to memory access to memory area no longer belonging to
the object and this a) wrong, b) raises a red flag in thread-safety tools.
The isc_mem API now crashes on memory allocation failure, and this is
the next commit in series to cleanup the code that could fail before,
but cannot fail now, e.g. isc_result_t return type has been changed to
void for the isc_log API functions that could only return ISC_R_SUCCESS.
A data race was happening while BIND was starting due to
isc_log_wouldlog function accessing lctx->logconfig without a lock.
To prevent that without incurring much costs, that variable was made
atomic.
Both clang-tidy and uncrustify chokes on statement like this:
for (...)
if (...)
break;
This commit uses a very simple semantic patch (below) to add braces around such
statements.
Semantic patch used:
@@
statement S;
expression E;
@@
while (...)
- if (E) S
+ { if (E) { S } }
@@
statement S;
expression E;
@@
for (...;...;...)
- if (E) S
+ { if (E) { S } }
@@
statement S;
expression E;
@@
if (...)
- if (E) S
+ { if (E) { S } }
Also disable the semantic patch as the code needs tweaks here and there because
some destroy functions might not destroy the object and return early if the
object is still in use.
389 else
CID 1452695 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking lcfg suggests that it may
be null, but it has already been dereferenced on all paths
leading to the check.
390 if (lcfg != NULL)
391 isc_logconfig_destroy(&lcfg);
4579. [func] Logging channels and dnstap output files can now
be configured with a "suffix" option, set to
either "increment" or "timestamp", indicating
whether to use incrementing numbers or timestamps
as the file suffix when rolling over a log file.
[RT #42838]
4518. [func] The "print-time" option in the logging configuration
can now take arguments "local", "iso8601" or
"iso8601-utc" to indicate the format in which the
date and time should be logged. For backward
compatibility, "yes" is a synonym for "local".
[RT #42585]
4411. [func] "rndc dnstap -roll" automatically rolls the
dnstap output file; the previous version is
saved with ".0" suffix, and earlier versions
with ".1" and so on. An optional numeric argument
indicates how many prior files to save. [RT #42830]
3741. [func] "delve" (domain entity lookup and validation engine):
A new tool with dig-like semantics for performing DNS
lookups, with internal DNSSEC validation, using the
same resolver and validator logic as named. This
allows easy validation of DNSSEC data in environments
with untrustworthy resolvers, and assists with
troubleshooting of DNSSEC problems. (Note: not yet
available on win32.) [RT #32406]
