resolved: take particular care when detaching DnsServer from its default stream

DnsStream and DnsServer have a symbiotic relationship: one DnsStream is
the current "default" stream of the server (and thus reffed by it), but
each stream also refs the server it is connected to. This cyclic
dependency can result in weird situations: when one is
destroyed/unlinked/stopped it needs to unregister itself from the other,
but doing this will trigger unregistration of the other. Hence, let's
make sure we unregister the stream from the server before destroying it,
to break this cycle.

Most likely fixes: #10725

diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index c7f9de2cbd9a3b34d54043d88c303d5bcc6a5feb..3e69741b8809bb71776af87da558684d76eaf870 100644 (file)
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -103,7 +103,7 @@ int dns_server_new(
 static DnsServer* dns_server_free(DnsServer *s)  {
         assert(s);
 
-        dns_stream_unref(s->stream);
+        dns_server_unref_stream(s);
 
 #if ENABLE_DNS_OVER_TLS
         dnstls_server_free(s);
@@ -158,6 +158,9 @@ void dns_server_unlink(DnsServer *s) {
         if (s->manager->current_dns_server == s)
                 manager_set_dns_server(s->manager, NULL);
 
+        /* No need to keep a default stream around anymore */
+        dns_server_unref_stream(s);
+
         dns_server_unref(s);
 }
 
@@ -826,6 +829,9 @@ void dns_server_reset_features(DnsServer *s) {
         s->warned_downgrade = false;
 
         dns_server_reset_counters(s);
+
+        /* Let's close the default stream, so that we reprobe with the new features */
+        dns_server_unref_stream(s);
 }
 
 void dns_server_reset_features_all(DnsServer *s) {
@@ -886,6 +892,20 @@ void dns_server_dump(DnsServer *s, FILE *f) {
                 yes_no(s->packet_rrsig_missing));
 }
 
+void dns_server_unref_stream(DnsServer *s) {
+        DnsStream *ref;
+
+        assert(s);
+
+        /* Detaches the default stream of this server. Some special care needs to be taken here, as that stream and
+         * this server reference each other. First, take the stream out of the server. It's destructor will check if it
+         * is registered with us, hence let's invalidate this separatly, so that it is already unregistered. */
+        ref = TAKE_PTR(s->stream);
+
+        /* And then, unref it */
+        dns_stream_unref(ref);
+}
+
 static const char* const dns_server_type_table[_DNS_SERVER_TYPE_MAX] = {
         [DNS_SERVER_SYSTEM] = "system",
         [DNS_SERVER_FALLBACK] = "fallback",

diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index a6022ad97f59d947ca790aad315ef6a757b2a5d9..6e73f32df45e581ba8784d08edb392069ceb29fd 100644 (file)
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -151,3 +151,5 @@ void dns_server_reset_features(DnsServer *s);
 void dns_server_reset_features_all(DnsServer *s);
 
 void dns_server_dump(DnsServer *s, FILE *f);
+
+void dns_server_unref_stream(DnsServer *s);

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 73e6306ba1d025178bc054b67bd6070a8f2edb39..f29a68e444acac311cd55e1a85b4d0a71a577f3a 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -639,7 +639,7 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
 #endif
 
                 if (t->server) {
-                        dns_stream_unref(t->server->stream);
+                        dns_server_unref_stream(t->server);
                         t->server->stream = dns_stream_ref(s);
                         s->server = dns_server_ref(t->server);
                 }