Commit 718ec80e authored by Diego dos Santos Fronza's avatar Diego dos Santos Fronza

Merge branch...

Merge branch '1472-threadsanitizer-lock-order-inversion-potential-deadlock-dns_resolver_createfetch-vs' into 'master'

Resolve "ThreadSanitizer: lock-order-inversion (potential deadlock) - dns_resolver_createfetch vs. dns_resolver_prime"

Closes #1472

See merge request !3020
parents a04cdde4 45543da8
Pipeline #34195 failed with stages
in 46 minutes and 42 seconds
......@@ -940,7 +940,8 @@ getrdata(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
do { \
if (best_effort) \
seen_problem = true; \
else { \
else \
{ \
result = r; \
goto cleanup; \
} \
......
......@@ -101,7 +101,8 @@ static unsigned char maptolower[] = {
#define SETUP_OFFSETS(name, var, default_offsets) \
if ((name)->offsets != NULL) \
var = (name)->offsets; \
else { \
else \
{ \
var = (default_offsets); \
set_offsets(name, var, NULL); \
}
......
......@@ -536,11 +536,11 @@ struct dns_resolver {
isc_refcount_t references;
atomic_uint_fast32_t zspill; /* fetches-per-zone */
atomic_bool exiting;
atomic_bool priming;
/* Locked by lock. */
isc_eventlist_t whenshutdown;
unsigned int activebuckets;
bool priming;
unsigned int spillat; /* clients-per-query */
dns_badcache_t *badcache; /* Bad cache. */
......@@ -9974,8 +9974,8 @@ destroy(dns_resolver_t *res) {
unsigned int i;
alternate_t *a;
REQUIRE(atomic_load(&res->references) == 0);
REQUIRE(!res->priming);
isc_refcount_destroy(&res->references);
REQUIRE(!atomic_load_acquire(&res->priming));
REQUIRE(res->primefetch == NULL);
RTRACE("destroy");
......@@ -10224,7 +10224,7 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
atomic_init(&res->exiting, false);
res->frozen = false;
ISC_LIST_INIT(res->whenshutdown);
res->priming = false;
atomic_init(&res->priming, false);
res->primefetch = NULL;
atomic_init(&res->nfctx, 0);
......@@ -10337,16 +10337,13 @@ prime_done(isc_task_t *task, isc_event_t *event) {
UNUSED(task);
LOCK(&res->lock);
INSIST(res->priming);
res->priming = false;
LOCK(&res->primelock);
fetch = res->primefetch;
res->primefetch = NULL;
UNLOCK(&res->primelock);
UNLOCK(&res->lock);
INSIST(atomic_compare_exchange_strong_acq_rel(&res->priming,
&(bool){ true }, false));
if (fevent->result == ISC_R_SUCCESS && res->view->cache != NULL &&
res->view->hints != NULL)
......@@ -10384,17 +10381,11 @@ dns_resolver_prime(dns_resolver_t *res) {
RTRACE("dns_resolver_prime");
LOCK(&res->lock);
/* XXXOND: cas needs to be used here */
if (!atomic_load_acquire(&res->exiting) && !res->priming) {
INSIST(res->primefetch == NULL);
res->priming = true;
want_priming = true;
if (!atomic_load_acquire(&res->exiting)) {
want_priming = atomic_compare_exchange_strong_acq_rel(
&res->priming, &(bool){ false }, true);
}
UNLOCK(&res->lock);
if (want_priming) {
/*
* To avoid any possible recursive locking problems, we
......@@ -10408,19 +10399,20 @@ dns_resolver_prime(dns_resolver_t *res) {
RTRACE("priming");
rdataset = isc_mem_get(res->mctx, sizeof(*rdataset));
dns_rdataset_init(rdataset);
LOCK(&res->primelock);
INSIST(res->primefetch == NULL);
result = dns_resolver_createfetch(
res, dns_rootname, dns_rdatatype_ns, NULL, NULL, NULL,
NULL, 0, DNS_FETCHOPT_NOFORWARD, 0, NULL,
res->buckets[0].task, prime_done, res, rdataset, NULL,
&res->primefetch);
UNLOCK(&res->primelock);
if (result != ISC_R_SUCCESS) {
isc_mem_put(res->mctx, rdataset, sizeof(*rdataset));
LOCK(&res->lock);
INSIST(res->priming);
res->priming = false;
UNLOCK(&res->lock);
INSIST(atomic_compare_exchange_strong_acq_rel(
&res->priming, &(bool){ true }, false));
}
inc_stats(res, dns_resstatscounter_priming);
}
......
......@@ -46,6 +46,9 @@
#define atomic_compare_exchange_strong_relaxed(o, e, d) \
atomic_compare_exchange_strong_explicit( \
(o), (e), (d), memory_order_relaxed, memory_order_relaxed)
#define atomic_compare_exchange_strong_acq_rel(o, e, d) \
atomic_compare_exchange_strong_explicit( \
(o), (e), (d), memory_order_acq_rel, memory_order_acquire)
/* Acquire-Release Memory Ordering */
......
......@@ -90,13 +90,15 @@
do { \
if ((elt)->link.next != NULL) \
(elt)->link.next->link.prev = (elt)->link.prev; \
else { \
else \
{ \
ISC_INSIST((list).tail == (elt)); \
(list).tail = (elt)->link.prev; \
} \
if ((elt)->link.prev != NULL) \
(elt)->link.prev->link.next = (elt)->link.next; \
else { \
else \
{ \
ISC_INSIST((list).head == (elt)); \
(list).head = (elt)->link.next; \
} \
......@@ -124,7 +126,8 @@
do { \
if ((before)->link.prev == NULL) \
ISC_LIST_PREPEND(list, elt, link); \
else { \
else \
{ \
(elt)->link.prev = (before)->link.prev; \
(before)->link.prev = (elt); \
(elt)->link.prev->link.next = (elt); \
......@@ -143,7 +146,8 @@
do { \
if ((after)->link.next == NULL) \
ISC_LIST_APPEND(list, elt, link); \
else { \
else \
{ \
(elt)->link.next = (after)->link.next; \
(after)->link.next = (elt); \
(elt)->link.next->link.prev = (elt); \
......@@ -162,7 +166,8 @@
do { \
if (ISC_LIST_EMPTY(list1)) \
(list1) = (list2); \
else if (!ISC_LIST_EMPTY(list2)) { \
else if (!ISC_LIST_EMPTY(list2)) \
{ \
(list1).tail->link.next = (list2).head; \
(list2).head->link.prev = (list1).tail; \
(list1).tail = (list2).tail; \
......@@ -175,7 +180,8 @@
do { \
if (ISC_LIST_EMPTY(list1)) \
(list1) = (list2); \
else if (!ISC_LIST_EMPTY(list2)) { \
else if (!ISC_LIST_EMPTY(list2)) \
{ \
(list2).tail->link.next = (list1).head; \
(list1).head->link.prev = (list2).tail; \
(list1).head = (list2).head; \
......
......@@ -85,7 +85,8 @@
GET8(v, w); \
if (v == 0) \
d = ISCCC_TRUE; \
else { \
else \
{ \
d = ISCCC_FALSE; \
if (v == 255) \
GET16(v, w); \
......@@ -165,7 +166,8 @@
do { \
if (v > 0 && v < 255) \
PUT8(v, w); \
else { \
else \
{ \
PUT8(255, w); \
PUT16(v, w); \
} \
......@@ -175,7 +177,8 @@
do { \
if (v < 0xffffffU) \
PUT24(v, w); \
else { \
else \
{ \
PUT24(0xffffffU, w); \
PUT32(v, w); \
} \
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment