Merge pull request #18407 from keszybz/resolved-reference-counting-again

Use reference counting for DnsQueryCandidate

3 files changed, 32 insertions, 33 deletions

diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index 757fbedf99..8ee4fd8309 100644
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -26,6 +26,7 @@ static int dns_query_candidate_new(DnsQueryCandidate **ret, DnsQuery *q, DnsScop
                 return -ENOMEM;
 
         *c = (DnsQueryCandidate) {
+                .n_ref = 1,
                 .query = q,
                 .scope = s,
         };
@@ -49,8 +50,7 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) {
         }
 }
 
-DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) {
-
+static DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) {
         if (!c)
                 return NULL;
 
@@ -68,8 +68,10 @@ DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) {
         return mfree(c);
 }
 
+DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(DnsQueryCandidate, dns_query_candidate, dns_query_candidate_free);
+
 static int dns_query_candidate_next_search_domain(DnsQueryCandidate *c) {
-        DnsSearchDomain *next = NULL;
+        DnsSearchDomain *next;
 
         assert(c);
 
@@ -130,14 +132,15 @@ static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResource
 }
 
 static int dns_query_candidate_go(DnsQueryCandidate *c) {
+        _cleanup_(dns_query_candidate_unrefp) DnsQueryCandidate *keep_c = NULL;
         DnsTransaction *t;
         int r;
         unsigned n = 0;
-        bool notify = false;
 
         assert(c);
 
-        c->query->block_ready++;
+        /* Let's keep a reference to the query while we're operating */
+        keep_c = dns_query_candidate_ref(c);
 
         /* Start the transactions that are not started yet */
         SET_FOREACH(t, c->transactions) {
@@ -145,21 +148,14 @@ static int dns_query_candidate_go(DnsQueryCandidate *c) {
                         continue;
 
                 r = dns_transaction_go(t);
-                if (r < 0) {
-                        c->query->block_ready--;
+                if (r < 0)
                         return r;
-                }
-                if (r == 0)
-                        /* A transaction is complete. */
-                        notify = true;
 
                 n++;
         }
 
-        c->query->block_ready--;
-
         /* If there was nothing to start, then let's proceed immediately */
-        if (n == 0 || notify)
+        if (n == 0)
                 dns_query_candidate_notify(c);
 
         return 0;
@@ -307,11 +303,11 @@ static void dns_query_stop(DnsQuery *q) {
                 dns_query_candidate_stop(c);
 }
 
-static void dns_query_free_candidates(DnsQuery *q) {
+static void dns_query_unref_candidates(DnsQuery *q) {
         assert(q);
 
         while (q->candidates)
-                dns_query_candidate_free(q->candidates);
+                dns_query_candidate_unref(q->candidates);
 }
 
 static void dns_query_reset_answer(DnsQuery *q) {
@@ -340,7 +336,7 @@ DnsQuery *dns_query_free(DnsQuery *q) {
                 LIST_REMOVE(auxiliary_queries, q->auxiliary_for->auxiliary_queries, q);
         }
 
-        dns_query_free_candidates(q);
+        dns_query_unref_candidates(q);
 
         dns_question_unref(q->question_idna);
         dns_question_unref(q->question_utf8);
@@ -515,7 +511,7 @@ static int on_query_timeout(sd_event_source *s, usec_t usec, void *userdata) {
 }
 
 static int dns_query_add_candidate(DnsQuery *q, DnsScope *s) {
-        _cleanup_(dns_query_candidate_freep) DnsQueryCandidate *c = NULL;
+        _cleanup_(dns_query_candidate_unrefp) DnsQueryCandidate *c = NULL;
         int r;
 
         assert(q);
@@ -602,8 +598,8 @@ static int dns_query_try_etc_hosts(DnsQuery *q) {
 
         assert(q);
 
-        /* Looks in /etc/hosts for matching entries. Note that this is done *before* the normal lookup is done. The
-         * data from /etc/hosts hence takes precedence over the network. */
+        /* Looks in /etc/hosts for matching entries. Note that this is done *before* the normal lookup is
+         * done. The data from /etc/hosts hence takes precedence over the network. */
 
         r = manager_etc_hosts_lookup(
                         q->manager,
@@ -936,7 +932,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
         dns_question_unref(q->question_utf8);
         q->question_utf8 = TAKE_PTR(nq_utf8);
 
-        dns_query_free_candidates(q);
+        dns_query_unref_candidates(q);
         dns_query_reset_answer(q);
 
         q->state = DNS_TRANSACTION_NULL;
diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h
index 8056aa3727..133076dbf0 100644
--- a/src/resolve/resolved-dns-query.h
+++ b/src/resolve/resolved-dns-query.h
@@ -16,12 +16,14 @@ typedef struct DnsStubListenerExtra DnsStubListenerExtra;
 #include "resolved-dns-transaction.h"
 
 struct DnsQueryCandidate {
+        unsigned n_ref;
+        int error_code;
+
         DnsQuery *query;
         DnsScope *scope;
 
         DnsSearchDomain *search_domain;
 
-        int error_code;
         Set *transactions;
 
         LIST_FIELDS(DnsQueryCandidate, candidates_by_query);
@@ -31,19 +33,19 @@ struct DnsQueryCandidate {
 struct DnsQuery {
         Manager *manager;
 
-        /* When resolving a service, we first create a TXT+SRV query,
-         * and then for the hostnames we discover auxiliary A+AAAA
-         * queries. This pointer always points from the auxiliary
-         * queries back to the TXT+SRV query. */
+        /* When resolving a service, we first create a TXT+SRV query, and then for the hostnames we discover
+         * auxiliary A+AAAA queries. This pointer always points from the auxiliary queries back to the
+         * TXT+SRV query. */
         DnsQuery *auxiliary_for;
         LIST_HEAD(DnsQuery, auxiliary_queries);
         unsigned n_auxiliary_queries;
         int auxiliary_result;
 
-        /* The question, formatted in IDNA for use on classic DNS, and as UTF8 for use in LLMNR or mDNS. Note that even
-         * on classic DNS some labels might use UTF8 encoding. Specifically, DNS-SD service names (in contrast to their
-         * domain suffixes) use UTF-8 encoding even on DNS. Thus, the difference between these two fields is mostly
-         * relevant only for explicit *hostname* lookups as well as the domain suffixes of service lookups. */
+        /* The question, formatted in IDNA for use on classic DNS, and as UTF8 for use in LLMNR or mDNS. Note
+         * that even on classic DNS some labels might use UTF8 encoding. Specifically, DNS-SD service names
+         * (in contrast to their domain suffixes) use UTF-8 encoding even on DNS. Thus, the difference
+         * between these two fields is mostly relevant only for explicit *hostname* lookups as well as the
+         * domain suffixes of service lookups. */
         DnsQuestion *question_idna;
         DnsQuestion *question_utf8;
 
@@ -101,8 +103,9 @@ enum {
         DNS_QUERY_RESTARTED,
 };
 
-DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c);
-DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQueryCandidate*, dns_query_candidate_free);
+DnsQueryCandidate* dns_query_candidate_ref(DnsQueryCandidate*);
+DnsQueryCandidate* dns_query_candidate_unref(DnsQueryCandidate*);
+DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQueryCandidate*, dns_query_candidate_unref);
 
 void dns_query_candidate_notify(DnsQueryCandidate *c);
 
diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 298ce21ae3..d77e81ae39 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -105,7 +105,7 @@ DnsScope* dns_scope_free(DnsScope *s) {
         dns_scope_abort_transactions(s);
 
         while (s->query_candidates)
-                dns_query_candidate_free(s->query_candidates);
+                dns_query_candidate_unref(s->query_candidates);
 
         hashmap_free(s->transactions_by_key);