BIND merge requestshttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests2023-04-03T09:19:56Zhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7608Suppress free, memset, and memcmp data races2023-04-03T09:19:56ZMichal NowakSuppress free, memset, and memcmp data racesThese data races are deemed harmless and should be suppressed.
As a validation, I ran the `doh_test` 500+ and 700+ times:
- [`unit:gcc:tsan`](https://gitlab.isc.org/isc-projects/bind9/-/jobs/3196367)
- [`unit:clang:tsan`](https://gitlab...These data races are deemed harmless and should be suppressed.
As a validation, I ran the `doh_test` 500+ and 700+ times:
- [`unit:gcc:tsan`](https://gitlab.isc.org/isc-projects/bind9/-/jobs/3196367)
- [`unit:clang:tsan`](https://gitlab.isc.org/isc-projects/bind9/-/jobs/3196368)
Closes #3265April 2023 (9.16.40, 9.16.40-S1, 9.18.14, 9.18.14-S1, 9.19.12)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7296Pin the dns_dispatch to threads when reusing2023-01-04T18:55:40ZOndřej SurýPin the dns_dispatch to threads when reusingPreviously, dns_dispatch_gettcp() could pick a TCP connection created by
different thread - this breaks our contractual promise to DNS dispatch
by using the TCP connection on a different thread than it was created.
Add .tid member to the...Previously, dns_dispatch_gettcp() could pick a TCP connection created by
different thread - this breaks our contractual promise to DNS dispatch
by using the TCP connection on a different thread than it was created.
Add .tid member to the dns_dispatch_t struct and skip the dispatches
from other threads when looking up a TCP dispatch that we can reuse in
dns_request.
NOTE: This is going to be properly refactored, but this change could be
also backported to 9.18 for better stability and thread-affinity.January 2023 (9.16.37, 9.16.37-S1, 9.18.11, 9.18.11-S1, 9.19.9)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/8586Resolve "Data races in isc_buffer_peekuint8, rdataset_settrust, and memmove"2024-03-06T09:07:53ZMatthijs Mekkingmatthijs@isc.orgResolve "Data races in isc_buffer_peekuint8, rdataset_settrust, and memmove"Enable the test that fails since it was revealed by !8515.
Closes #4475Enable the test that fails since it was revealed by !8515.
Closes #4475March 2024 (9.16.49, 9.16.49-S1, 9.18.25, 9.18.25-S1, 9.19.22)Matthijs Mekkingmatthijs@isc.orgMatthijs Mekkingmatthijs@isc.orghttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/8315Replace mempool_{create,destroy} locking with RCU list2023-10-04T13:26:10ZOndřej SurýReplace mempool_{create,destroy} locking with RCU listAccording to the measurements, we spend quite a lot of time waiting for
the memory context lock in isc_mempool_{create,destroy}. Remove the
memory context lock, replace the mempool list with RCU list, and don't
protect the .checkfree an...According to the measurements, we spend quite a lot of time waiting for
the memory context lock in isc_mempool_{create,destroy}. Remove the
memory context lock, replace the mempool list with RCU list, and don't
protect the .checkfree and .name variables - those are constant for the
lifetime of the memory context anyway.
Closes #4325November 2023 (9.16.45, 9.16.45-S1, 9.18.20, 9.18.20-S1, 9.19.18)Evan HuntEvan Hunthttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7645Add isc_spinlock unit with shim pthread_spin implementation2023-04-03T09:34:37ZOndřej SurýAdd isc_spinlock unit with shim pthread_spin implementationThe spinlock is small (atomic_uint_fast32_t at most), lightweight
synchronization primitive and should only be used for short-lived and
most of the time a isc_mutex should be used.
Add a isc_spinlock unit which is either (most of the ti...The spinlock is small (atomic_uint_fast32_t at most), lightweight
synchronization primitive and should only be used for short-lived and
most of the time a isc_mutex should be used.
Add a isc_spinlock unit which is either (most of the time) a think
wrapper around pthread_spin API or an efficient shim implementation of
the simple spinlock.April 2023 (9.16.40, 9.16.40-S1, 9.18.14, 9.18.14-S1, 9.19.12)Ondřej SurýOndřej Surýhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7561Draft: Use atomic stack for async job queue2023-02-22T16:25:44ZOndřej SurýDraft: Use atomic stack for async job queuePreviously, the async job queue would use a locked-list (ISC_LIST).
With introduction of atomic stack (that has to be drained at once), we
could use it to remove some contention between the threads and simplify
the async queue.
Fortunat...Previously, the async job queue would use a locked-list (ISC_LIST).
With introduction of atomic stack (that has to be drained at once), we
could use it to remove some contention between the threads and simplify
the async queue.
Fortunately, the reverse order still works for us - instead of append
and tail/prev operation on the list, we are now using prepend and
head/next operation on the atomic stack.March 2023 (9.16.39, 9.16.39-S1, 9.18.13, 9.18.13-S1, 9.19.11)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/7472Refactor qp-trie to use QSBR2023-02-27T12:22:44ZTony FinchRefactor qp-trie to use QSBRThe first working multi-threaded qp-trie was stuck with an unpleasant
trade-off:
* Use `isc_rwlock`, which has acceptable write performance, but
terrible read scalability because the qp-trie made all accesses
through a single lock.
* U...The first working multi-threaded qp-trie was stuck with an unpleasant
trade-off:
* Use `isc_rwlock`, which has acceptable write performance, but
terrible read scalability because the qp-trie made all accesses
through a single lock.
* Use `liburcu`, which has great read scalability, but terrible
write performance, because I was relying on `rcu_synchronize()`
which is rather slow. And `liburcu` is LGPL.
To get the best of both worlds, we need our own scalable read side,
which we now have with `isc_qsbr`. And we need to modify the write
side so that it is not blocked by readers.
Better write performance requires an async cleanup function like
`call_rcu()`, instead of the blocking `rcu_synchronize()`. (There
is no blocking cleanup in `isc_qsbr`, because I have concluded
that it would be an attractive nuisance.)
Until now, all my multithreading qp-trie designs have been based
around two versions, read-only and mutable. This is too few to
work with asynchronous cleanup. The bare minimum (as in epoch
based reclamation) is three, but it makes more sense to support an
arbitrary number. Doing multi-version support "properly" makes
fewer assumptions about how safe memory reclamation works, and it
makes snapshots and rollbacks simpler.
To avoid making the memory management even more complicated, I
have introduced a new kind of "packed reader node" to anchor the
root of a version of the trie. This is simpler because it re-uses
the existing chunk lifetime logic - see the discussion under
"packed reader nodes" in `qp_p.h`.
I have also made the chunk lifetime logic simpler. The idea of a
"generation" is gone; instead, chunks are either mutable or
immutable. And the QSBR phase number is used to indicate when a
chunk can be reclaimed.
Instead of the `shared_base` flag (which was basically a one-bit
reference count, with a two version limit) the base array now has a
refcount, which replaces the confusing ad-hoc lifetime logic with
something more familiar and systematic.March 2023 (9.16.39, 9.16.39-S1, 9.18.13, 9.18.13-S1, 9.19.11)Tony FinchTony Finchhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/6211Support for thread sanitizer builds on macOS2023-01-11T09:27:42ZTony FinchSupport for thread sanitizer builds on macOSThread sanitizer builds normally use pthread_barrier instead of
uv_barrier in order to get a sanitized implementation. However,
macOS lacks pthread_barrier, so now tsan builds on macOS use
uv_barrier like normal builds, assuming that lib...Thread sanitizer builds normally use pthread_barrier instead of
uv_barrier in order to get a sanitized implementation. However,
macOS lacks pthread_barrier, so now tsan builds on macOS use
uv_barrier like normal builds, assuming that libuv has been built
with tsan support so that thread sanitizer can see inside it.January 2023 (9.16.37, 9.16.37-S1, 9.18.11, 9.18.11-S1, 9.19.9)Tony FinchTony Finchhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/8019Improve locking in zone_maintenance()2023-06-14T13:30:30ZTony FinchImprove locking in zone_maintenance()Thread sanitizer reported a data race involving a zone timer,
because zone_maintenance() was not careful about consistently
locking the zone when accessing its timers. Fix this by adding a
helper function that locks the zone while checki...Thread sanitizer reported a data race involving a zone timer,
because zone_maintenance() was not careful about consistently
locking the zone when accessing its timers. Fix this by adding a
helper function that locks the zone while checking one of its
timers, and adjust zone_maintenance() to use it where appropriate,
and to access other zone properties under a lock.
Closes #4135July 2023 (9.18.17, 9.18.17-S1, 9.19.15)Tony FinchTony Finchhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/4879Draft: Resolve "ThreadSanitizer: lock-order-inversion in pthread_mutex_lock"2022-01-17T11:17:44ZDiego dos Santos FronzaDraft: Resolve "ThreadSanitizer: lock-order-inversion in pthread_mutex_lock"Closes #2615Closes #2615February 2022 (9.16.26, 9.16.26-S1)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/4713Disable Thread Sanitizer in isc_dir_close()2021-02-19T11:52:10ZOndřej SurýDisable Thread Sanitizer in isc_dir_close()There's a false positive reported by Thread Sanitizer between the
epoll_ctl() called from libuv and closedir() called from libisc. The
Thread Sanitizer monitors file descriptors, but there's no possible way
the file descriptor in epoll_...There's a false positive reported by Thread Sanitizer between the
epoll_ctl() called from libuv and closedir() called from libisc. The
Thread Sanitizer monitors file descriptors, but there's no possible way
the file descriptor in epoll_ctl() could be the same as the directory
being closed(). Let's just disable the the Thread Sanitizer for
isc_dir_close() function to prevent reporting of this false positive.
Closes: #2457
WARNING: ThreadSanitizer: data race
Write of size 8 at 0x000000000001 by thread T1:
#0 closedir <null>
#1 isc_dir_close lib/isc/unix/dir.c:134
#2 dns_dnssec_findmatchingkeys lib/dns/dnssec.c:1526
#3 statefile_exist lib/dns/zone.c:5858
#4 dns_zone_secure_to_insecure lib/dns/zone.c:5905
#5 dns_zone_use_kasp lib/dns/zone.c:5917
#6 add_sigs lib/dns/zone.c:6901
#7 dns__zone_updatesigs lib/dns/zone.c:8225
#8 zone_nsec3chain lib/dns/zone.c:8921
#9 zone_maintenance lib/dns/zone.c:11211
#10 zone_timer lib/dns/zone.c:14674
#11 dispatch lib/isc/task.c:1152
#12 run lib/isc/task.c:1344
#13 <null> <null>
Previous read of size 8 at 0x000000000001 by thread T2:
#0 epoll_ctl <null>
#1 uv__platform_invalidate_fd <null>
#2 uv_run <null>
#3 <null> <null>
Location is file descriptor 143 created by thread T2 at:
#0 accept4 <null>
#1 uv__accept <null>
#2 <null> <null>
Thread T1 (running) created by main thread at:
#0 pthread_create <null>
#1 isc_thread_create lib/isc/pthreads/thread.c:73
#2 isc_taskmgr_create lib/isc/task.c:1434
#3 create_managers bin/named/main.c:940
#4 setup bin/named/main.c:1248
#5 main bin/named/main.c:1548
Thread T2 (running) created by main thread at:
#0 pthread_create <null>
#1 isc_thread_create lib/isc/pthreads/thread.c:73
#2 isc_nm_start lib/isc/netmgr/netmgr.c:290
#3 create_managers bin/named/main.c:934
#4 setup bin/named/main.c:1248
#5 main bin/named/main.c:1548
SUMMARY: ThreadSanitizer: data race in closedir
Closes #2457March 2021 (9.11.29, 9.11.29-S1, 9.16.13, 9.16.13-S1, 9.17.11)Ondřej SurýOndřej Surýhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/4215Add missing lock calls in isc__nm_tcpdns_stoplistening()2020-10-08T06:00:27ZMark AndrewsAdd missing lock calls in isc__nm_tcpdns_stoplistening()isc__nmsocket_clearcb() can update a socket being processed
on a different thread and without the lock calls tsan errors
can be produced between isc__nm_tcpdns_stoplistening() and
processbuffer().isc__nmsocket_clearcb() can update a socket being processed
on a different thread and without the lock calls tsan errors
can be produced between isc__nm_tcpdns_stoplistening() and
processbuffer().Mark AndrewsMark Andrewshttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/4214Draft: Resolve "TSAN errors in v9_16 to be investigated."2020-10-02T01:42:18ZMark AndrewsDraft: Resolve "TSAN errors in v9_16 to be investigated."Closes #2193Closes #2193November 2020 (9.11.25, 9.11.25-S1, 9.16.9, 9.16.9-S1, 9.17.7)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/4185Draft: Resolve "ThreadSanitizer: data race in dup"2020-12-03T11:03:39ZMark AndrewsDraft: Resolve "ThreadSanitizer: data race in dup"Closes #2161Closes #2161December 2020 (9.11.26, 9.11.26-S1, 9.16.10, 9.16.10-S1, 9.17.8)https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/3694WIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_...2020-07-16T07:44:07ZMark AndrewsWIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_stale and check_stale_header"Closes #1475Closes #1475August 2020 (9.11.22, 9.11.22-S1, 9.16.6, 9.17.4)Ondřej SurýOndřej Surýhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/3087WIP: Resolve "ThreadSanitizer: lock-order-inversion (potential deadlock) - va...2021-07-21T15:30:32ZDiego dos Santos FronzaWIP: Resolve "ThreadSanitizer: lock-order-inversion (potential deadlock) - validator_start vs. dns_resolver_createfetch"Closes #1470Closes #1470BIND 9.17 BackburnerOndřej SurýOndřej Surýhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/3775WIP: Resolve "ThreadSanitizer: data race (bin/named/.libs/named+0x446212) in ...2020-08-24T14:01:38ZDiego dos Santos FronzaWIP: Resolve "ThreadSanitizer: data race (bin/named/.libs/named+0x446212) in epoll_ctl"Closes #1484Closes #1484September 2020 (9.11.23, 9.11.23-S1, 9.16.7, 9.17.5)Diego dos Santos FronzaDiego dos Santos Fronzahttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/2908Sprinkle rbtdb.c with memory synchronization2021-03-22T10:30:42ZOndřej SurýSprinkle rbtdb.c with memory synchronizationCloses #1440Closes #1440BIND 9.17 BackburnerOndřej SurýOndřej Surýhttps://gitlab.isc.org/isc-projects/bind9/-/merge_requests/2797WIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_...2020-01-14T12:38:07ZMark AndrewsWIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_stale and check_stale_header"Closes #1475Closes #1475https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/2775WIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_...2020-07-02T04:53:54ZMark AndrewsWIP: Resolve "ThreadSanitizer: data race lib/dns/rbtdb.c:1545 in mark_header_stale and check_stale_header"Closes #1475Closes #1475BIND 9.17 Backburner