remove unnecessary assertion in dns_dispatch_connect()

When a thread calls dns_dispatch_connect() on an unconnected TCP socket
it sets `tcpstate` from `DNS_DISPATCHSTATE_NONE` to `_CONNECTING`.
Previously, it then INSISTed that there were no pending connections
before calling isc_nm_tcpdnsconnect().

If a second thread called dns_dispatch_connect() during that window
of time, it could add a pending connection to the list, and trigger
an assertion failure.

This commit removes the INSIST since the condition is actually
harmless.

(cherry picked from commit 25ddec8a0a8e0549b69f4e601028f3323d3c1886)

diff --git a/CHANGES b/CHANGES
index 12a7e5cc2aa89f1d25330c01ebc0596857569939..90c91df30d2b847390bebbf6c348d23c5eeaeb1c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+5927.  [bug]           A race was possible in dns_dispatch_connect()
+                       that could trigger an assertion failure if two
+                       threads called it near-simultaneously. [GL #3456]
+
 5926.  [func]          Handle transient TCP connect() EADDRINUSE failures
                        on FreeBSD (and possibly other BSDs) by trying three
                        times before giving up. [GL #3451]

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index f3386aab5781d7e87767bb0c17a3491f374349cc..8bc07b34d84e05b6c35fc8e2a8d739b39a07c1e5 100644 (file)
--- a/lib/dns/dispatch.c
+++ b/lib/dns/dispatch.c
@@ -1812,7 +1812,6 @@ dns_dispatch_connect(dns_dispentry_t *resp) {
                case DNS_DISPATCHSTATE_NONE:
                        /* First connection, continue with connecting */
                        LOCK(&disp->lock);
-                       INSIST(ISC_LIST_EMPTY(disp->pending));
                        ISC_LIST_APPEND(disp->pending, resp, plink);
                        UNLOCK(&disp->lock);
                        dns_dispatch_attach(disp, &(dns_dispatch_t *){ NULL });