summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-02-29 23:18:16 -0500
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-02-29 23:18:16 -0500
commit6014237390d5d9a20a274d4fcd9c84669cacca83 (patch)
tree5c676e1d5850f389968c222cc02715bf8303d96a /src
parent97a437d24cc97bc7132d3d3696b4c41e5a93edca (diff)
parent35aa04e9edf422beac3493afa555d29575b3046c (diff)
Merge pull request #2702 from poettering/resolved-iterate-fix
resolved iteration fix
Diffstat (limited to 'src')
-rw-r--r--src/basic/hashmap.c14
-rw-r--r--src/basic/macro.h6
-rw-r--r--src/basic/set.h3
-rw-r--r--src/resolve/resolved-dns-query.c5
-rw-r--r--src/resolve/resolved-dns-transaction.c62
-rw-r--r--src/resolve/resolved-dns-transaction.h6
-rw-r--r--src/resolve/resolved-dns-zone.c5
7 files changed, 59 insertions, 42 deletions
diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c
index 6f1a049d47..85b8d812b3 100644
--- a/src/basic/hashmap.c
+++ b/src/basic/hashmap.c
@@ -176,7 +176,7 @@ enum HashmapType {
};
struct _packed_ indirect_storage {
- char *storage; /* where buckets and DIBs are stored */
+ void *storage; /* where buckets and DIBs are stored */
uint8_t hash_key[HASH_KEY_SIZE]; /* hash key; changes during resize */
unsigned n_entries; /* number of stored entries */
@@ -193,7 +193,7 @@ struct direct_storage {
/* This gives us 39 bytes on 64bit, or 35 bytes on 32bit.
* That's room for 4 set_entries + 4 DIB bytes + 3 unused bytes on 64bit,
* or 7 set_entries + 7 DIB bytes + 0 unused bytes on 32bit. */
- char storage[sizeof(struct indirect_storage)];
+ uint8_t storage[sizeof(struct indirect_storage)];
};
#define DIRECT_BUCKETS(entry_t) \
@@ -302,7 +302,7 @@ static void n_entries_dec(HashmapBase *h) {
h->n_direct_entries--;
}
-static char *storage_ptr(HashmapBase *h) {
+static void *storage_ptr(HashmapBase *h) {
return h->has_indirect ? h->indirect.storage
: h->direct.storage;
}
@@ -347,7 +347,7 @@ static void get_hash_key(uint8_t hash_key[HASH_KEY_SIZE], bool reuse_is_ok) {
static struct hashmap_base_entry *bucket_at(HashmapBase *h, unsigned idx) {
return (struct hashmap_base_entry*)
- (storage_ptr(h) + idx * hashmap_type_info[h->type].entry_size);
+ ((uint8_t*) storage_ptr(h) + idx * hashmap_type_info[h->type].entry_size);
}
static struct plain_hashmap_entry *plain_bucket_at(Hashmap *h, unsigned idx) {
@@ -381,7 +381,7 @@ static struct hashmap_base_entry *bucket_at_virtual(HashmapBase *h, struct swap_
static dib_raw_t *dib_raw_ptr(HashmapBase *h) {
return (dib_raw_t*)
- (storage_ptr(h) + hashmap_type_info[h->type].entry_size * n_buckets(h));
+ ((uint8_t*) storage_ptr(h) + hashmap_type_info[h->type].entry_size * n_buckets(h));
}
static unsigned bucket_distance(HashmapBase *h, unsigned idx, unsigned from) {
@@ -1028,7 +1028,7 @@ static int hashmap_base_put_boldly(HashmapBase *h, unsigned idx,
*/
static int resize_buckets(HashmapBase *h, unsigned entries_add) {
struct swap_entries swap;
- char *new_storage;
+ void *new_storage;
dib_raw_t *old_dibs, *new_dibs;
const struct hashmap_type_info *hi;
unsigned idx, optimal_idx;
@@ -1095,7 +1095,7 @@ static int resize_buckets(HashmapBase *h, unsigned entries_add) {
h->indirect.n_buckets = (1U << new_shift) /
(hi->entry_size + sizeof(dib_raw_t));
- old_dibs = (dib_raw_t*)(new_storage + hi->entry_size * old_n_buckets);
+ old_dibs = (dib_raw_t*)((uint8_t*) new_storage + hi->entry_size * old_n_buckets);
new_dibs = dib_raw_ptr(h);
/*
diff --git a/src/basic/macro.h b/src/basic/macro.h
index ddf0968d1b..c34441d75d 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 a46674f6fe..706f8c14ed 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 57f9455131..a5129c201e 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)
- dns_zone_item_notify(z);
+ SWAP_TWO(t->notify_query_candidates, t->notify_query_candidates_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));
+ 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);
- 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;