diff --git a/src/basic/macro.h b/src/basic/macro.h index 2695d0edb7..ab5cc97e17 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -361,6 +361,12 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) { _found; \ }) +#define SWAP_TWO(x, y) do { \ + typeof(x) _t = (x); \ + (x) = (y); \ + (y) = (_t); \ + } while (false) + /* Define C11 thread_local attribute even on older gcc compiler * version */ #ifndef thread_local diff --git a/src/basic/set.h b/src/basic/set.h index 2bff5062da..e0d9dd001c 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -126,6 +126,9 @@ int set_put_strdupv(Set *s, char **l); #define SET_FOREACH(e, s, i) \ for ((i) = ITERATOR_FIRST; set_iterate((s), &(i), (void**)&(e)); ) +#define SET_FOREACH_MOVE(e, d, s) \ + for (; ({ e = set_first(s); assert_se(!e || set_move_one(d, s, e) >= 0); e; }); ) + DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free); DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free); diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index a7496aa586..dd133e1097 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -62,6 +62,7 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) { while ((t = set_steal_first(c->transactions))) { set_remove(t->notify_query_candidates, c); + set_remove(t->notify_query_candidates_done, c); dns_transaction_gc(t); } } @@ -139,6 +140,10 @@ static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResource if (r < 0) goto gc; + r = set_ensure_allocated(&t->notify_query_candidates_done, NULL); + if (r < 0) + goto gc; + r = set_put(t->notify_query_candidates, c); if (r < 0) goto gc; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 3443f71976..1cd5d1be8a 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -52,6 +52,7 @@ static void dns_transaction_flush_dnssec_transactions(DnsTransaction *t) { while ((z = set_steal_first(t->dnssec_transactions))) { set_remove(z->notify_transactions, t); + set_remove(z->notify_transactions_done, t); dns_transaction_gc(z); } } @@ -100,14 +101,26 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { set_remove(c->transactions, t); set_free(t->notify_query_candidates); + while ((c = set_steal_first(t->notify_query_candidates_done))) + set_remove(c->transactions, t); + set_free(t->notify_query_candidates_done); + while ((i = set_steal_first(t->notify_zone_items))) i->probe_transaction = NULL; set_free(t->notify_zone_items); + while ((i = set_steal_first(t->notify_zone_items_done))) + i->probe_transaction = NULL; + set_free(t->notify_zone_items_done); + while ((z = set_steal_first(t->notify_transactions))) set_remove(z->dnssec_transactions, t); set_free(t->notify_transactions); + while ((z = set_steal_first(t->notify_transactions_done))) + set_remove(z->dnssec_transactions, t); + set_free(t->notify_transactions_done); + dns_transaction_flush_dnssec_transactions(t); set_free(t->dnssec_transactions); @@ -127,8 +140,11 @@ bool dns_transaction_gc(DnsTransaction *t) { return true; if (set_isempty(t->notify_query_candidates) && + set_isempty(t->notify_query_candidates_done) && set_isempty(t->notify_zone_items) && - set_isempty(t->notify_transactions)) { + set_isempty(t->notify_zone_items_done) && + set_isempty(t->notify_transactions) && + set_isempty(t->notify_transactions_done)) { dns_transaction_free(t); return false; } @@ -266,6 +282,7 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { log_debug("We have the lexicographically larger IP address and thus lost in the conflict."); t->block_gc++; + while ((z = set_first(t->notify_zone_items))) { /* First, make sure the zone item drops the reference * to us */ @@ -284,7 +301,6 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { DnsQueryCandidate *c; DnsZoneItem *z; DnsTransaction *d; - Iterator i; const char *st; char key_str[DNS_RESOURCE_KEY_STRING_MAX]; @@ -333,39 +349,17 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { * transaction isn't freed while we are still looking at it */ t->block_gc++; - SET_FOREACH(c, t->notify_query_candidates, i) + SET_FOREACH_MOVE(c, t->notify_query_candidates_done, t->notify_query_candidates) dns_query_candidate_notify(c); - SET_FOREACH(z, t->notify_zone_items, i) + SWAP_TWO(t->notify_query_candidates, t->notify_query_candidates_done); + + SET_FOREACH_MOVE(z, t->notify_zone_items_done, t->notify_zone_items) dns_zone_item_notify(z); + SWAP_TWO(t->notify_zone_items, t->notify_zone_items_done); - if (!set_isempty(t->notify_transactions)) { - DnsTransaction **nt; - unsigned j, n = 0; - - /* We need to be careful when notifying other - * transactions, as that might destroy other - * transactions in our list. Hence, in order to be - * able to safely iterate through the list of - * transactions, take a GC lock on all of them - * first. Then, in a second loop, notify them, but - * first unlock that specific transaction. */ - - nt = newa(DnsTransaction*, set_size(t->notify_transactions)); - SET_FOREACH(d, t->notify_transactions, i) { - nt[n++] = d; - d->block_gc++; - } - - assert(n == set_size(t->notify_transactions)); - - for (j = 0; j < n; j++) { - if (set_contains(t->notify_transactions, nt[j])) - dns_transaction_notify(nt[j], t); - - nt[j]->block_gc--; - dns_transaction_gc(nt[j]); - } - } + SET_FOREACH_MOVE(d, t->notify_transactions_done, t->notify_transactions) + dns_transaction_notify(d, t); + SWAP_TWO(t->notify_transactions, t->notify_transactions_done); t->block_gc--; dns_transaction_gc(t); @@ -1626,6 +1620,10 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource if (r < 0) goto gc; + r = set_ensure_allocated(&aux->notify_transactions_done, NULL); + if (r < 0) + goto gc; + r = set_put(t->dnssec_transactions, aux); if (r < 0) goto gc; diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 491c62d772..eaece91533 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -118,17 +118,17 @@ struct DnsTransaction { /* Query candidates this transaction is referenced by and that * shall be notified about this specific transaction * completing. */ - Set *notify_query_candidates; + Set *notify_query_candidates, *notify_query_candidates_done; /* Zone items this transaction is referenced by and that shall * be notified about completion. */ - Set *notify_zone_items; + Set *notify_zone_items, *notify_zone_items_done; /* Other transactions that this transactions is referenced by * and that shall be notified about completion. This is used * when transactions want to validate their RRsets, but need * another DNSKEY or DS RR to do so. */ - Set *notify_transactions; + Set *notify_transactions, *notify_transactions_done; /* The opposite direction: the transactions this transaction * created in order to request DNSKEY or DS RRs. */ diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 03813da6a2..850eed8cb8 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -38,6 +38,7 @@ void dns_zone_item_probe_stop(DnsZoneItem *i) { i->probe_transaction = NULL; set_remove(t->notify_zone_items, i); + set_remove(t->notify_zone_items_done, i); dns_transaction_gc(t); } @@ -186,6 +187,10 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { if (r < 0) goto gc; + r = set_ensure_allocated(&t->notify_zone_items_done, NULL); + if (r < 0) + goto gc; + r = set_put(t->notify_zone_items, i); if (r < 0) goto gc;