Added test for the fix

This test ensures that named will correctly shutdown
when receiving multiple control connections after processing
of either "rncd stop" or "kill -SIGTERM" commands.

Before the fix, named was crashing due to a race condition happening
between two threads, one running shutdown logic in named/server.c
and other handling control logic in controlconf.c.

This test tries to reproduce the above scenario by issuing multiple
queries to a target named instance, issuing either rndc stop or kill
-SIGTERM command to the same named instance, then starting multiple rndc
status connections to ensure it is not crashing anymore.

bin/tests/system/Makefile.am

@@ -76,7 +76,7 @@ TESTS +=		\
 	rpzrecurse
 endif HAVE_PERLMOD_NET_DNS
 
-TESTS += 			\
+TESTS +=			\
 	acl			\
 	additional		\
 	addzone			\
@@ -107,7 +107,7 @@ TESTS += 			\
 	geoip2			\
 	glue			\
 	idna			\
-	include-multiplecfg 	\
+	include-multiplecfg	\
 	inline			\
 	integrity		\
 	kasp			\
@@ -134,6 +134,7 @@ TESTS += 			\
 	rrsetorder		\
 	rsabigexponent		\
 	runtime			\
+	shutdown		\
 	sfcache			\
 	smartsign		\
 	sortlist		\

bin/tests/system/conf.sh.common

@@ -114,6 +114,7 @@ rrsetorder
 rsabigexponent
 runtime
 sfcache
+shutdown
 smartsign
 sortlist
 spf

bin/tests/system/shutdown/clean.sh

@@ -0,0 +1,18 @@
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+rm -f ns*/*.jnl
+rm -f ns*/named.conf
+rm -f ns*/named.lock
+rm -f ns*/named.memstats
+rm -f ns*/named.run
+rm -f ns*/rpz*.txt
+rm -f resolver/named.conf
+rm -rf __pycache__
+rm -f *.status

bin/tests/system/shutdown/conftest.py

@@ -0,0 +1,58 @@
+############################################################################
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+############################################################################
+
+import os
+import pytest
+
+
+def pytest_configure(config):
+    config.addinivalue_line(
+        "markers", "dnspython: mark tests that need dnspython to function"
+    )
+
+
+def pytest_collection_modifyitems(config, items):
+    # pylint: disable=unused-argument,unused-import,too-many-branches
+    # pylint: disable=import-outside-toplevel
+
+    # Test for dnspython module
+    skip_dnspython = pytest.mark.skip(
+        reason="need dnspython module to run")
+    try:
+        import dns.query  # noqa: F401
+    except ModuleNotFoundError:
+        for item in items:
+            if "dnspython" in item.keywords:
+                item.add_marker(skip_dnspython)
+
+
+@pytest.fixture
+def named_port(request):
+    # pylint: disable=unused-argument
+    port = os.getenv("PORT")
+    if port is None:
+        port = 5301
+    else:
+        port = int(port)
+
+    return port
+
+
+@pytest.fixture
+def control_port(request):
+    # pylint: disable=unused-argument
+    port = os.getenv("CONTROLPORT")
+    if port is None:
+        port = 5301
+    else:
+        port = int(port)
+
+    return port

bin/tests/system/shutdown/ns1/named.conf.in

@@ -0,0 +1,29 @@
+key rndc_key {
+    secret "1234abcd8765";
+	algorithm hmac-sha256;
+};
+
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+options {
+	query-source address 10.53.0.1;
+	notify-source 10.53.0.1;
+	transfer-source 10.53.0.1;
+	port @PORT@;
+	listen-on { 10.53.0.1; };
+	pid-file "named.pid";
+	notify no;
+	dnssec-validation no;
+	allow-query { any; };
+	recursion yes;
+	allow-recursion { any; };
+};
+
+# Delegate .test domain to 10.53.0.2
+zone "." {
+    type master;
+    file "root.db";
+    allow-transfer { none; };
+};

bin/tests/system/shutdown/ns1/root.db

@@ -0,0 +1,23 @@
+; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+;
+; This Source Code Form is subject to the terms of the Mozilla Public
+; License, v. 2.0. If a copy of the MPL was not distributed with this
+; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;
+; See the COPYRIGHT file distributed with this work for additional
+; information regarding copyright ownership.
+
+$TTL 300
+@ 			IN SOA	a.root. root.test. (
+				2000042100   	; serial
+				600         	; refresh
+				600         	; retry
+				1200    	; expire
+				600       	; minimum
+				)
+
+.         IN   NS a.root.
+a.root.   IN   A  10.53.0.1
+
+test      IN   NS   ns1.test
+ns1.test  IN   A    10.53.0.2

bin/tests/system/shutdown/ns2/named.conf.in

@@ -0,0 +1,27 @@
+key rndc_key {
+    secret "1234abcd8765";
+	algorithm hmac-sha256;
+};
+
+controls {
+	inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+options {
+	query-source address 10.53.0.2;
+	notify-source 10.53.0.2;
+	transfer-source 10.53.0.2;
+	port @PORT@;
+	listen-on { 10.53.0.2; };
+	pid-file "named.pid";
+	notify no;
+	dnssec-validation no;
+	allow-query { any; };
+};
+
+# 10.53.0.2 is authoritative for .test domain
+zone "test" {
+    type master;
+	file "test.db";
+	allow-transfer { none; };
+};

bin/tests/system/shutdown/ns2/test.db

@@ -0,0 +1,7 @@
+$TTL 300
+
+@	IN	SOA	ns1 root.test. 2020040101 4h 1h 1w 60
+@	IN	NS	ns1
+ns1	IN	A	10.53.0.2
+@	IN	A	10.53.0.2
+www	IN	A	10.53.0.2

bin/tests/system/shutdown/resolver/named.conf.in

@@ -0,0 +1,26 @@
+key rndc_key {
+    secret "1234abcd8765";
+	algorithm hmac-sha256;
+};
+
+controls {
+	inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+options {
+	query-source address 10.53.0.3;
+	notify-source 10.53.0.3;
+	transfer-source 10.53.0.3;
+	port @PORT@;
+	listen-on { 10.53.0.3; };
+	pid-file "named.pid";
+	notify no;
+	dnssec-validation no;
+	allow-query { any; };
+	allow-recursion { any; };
+};
+
+zone "." {
+	type hint;
+	file "root.db";
+};

bin/tests/system/shutdown/resolver/root.db

@@ -0,0 +1,19 @@
+; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+;
+; This Source Code Form is subject to the terms of the Mozilla Public
+; License, v. 2.0. If a copy of the MPL was not distributed with this
+; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;
+; See the COPYRIGHT file distributed with this work for additional
+; information regarding copyright ownership.
+
+$TTL 300
+. 			IN SOA	a.root. root.root. (
+				2000042100   	; serial
+				600         	; refresh
+				600         	; retry
+				1200    	; expire
+				600       	; minimum
+				)
+.         IN   NS  a.root.
+a.root.   IN   A   10.53.0.1

bin/tests/system/shutdown/setup.sh

@@ -0,0 +1,21 @@
+#! /bin/sh
+#
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+
+# touch dnsrps-off to not test with DNSRPS
+
+set -e
+
+SYSTEMTESTTOP=..
+. $SYSTEMTESTTOP/conf.sh
+
+copy_setports ns1/named.conf.in ns1/named.conf
+copy_setports ns2/named.conf.in ns2/named.conf
+copy_setports resolver/named.conf.in resolver/named.conf

bin/tests/system/shutdown/tests-shutdown.py

@@ -0,0 +1,193 @@
+#!/usr/bin/python3
+############################################################################
+# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# See the COPYRIGHT file distributed with this work for additional
+# information regarding copyright ownership.
+############################################################################
+
+from concurrent.futures import ThreadPoolExecutor, as_completed
+import os
+import random
+import subprocess
+from string import ascii_lowercase as letters
+import time
+
+import dns.resolver
+import pytest
+
+
+def do_work(named_proc, resolver, rndc_cmd, kill_method, n_workers, n_queries):
+    """Creates a number of A queries to run in parallel
+       in order simulate a slightly more realistic test scenario.
+
+       The main idea of this function is to create and send a bunch
+       of A queries to a target named instance and during this process
+       a request for shutting down named will be issued.
+
+       In the process of shutting down named, a couple control connections
+       are created (by launching rndc) to ensure that the crash was fixed.
+
+       if kill_method=="rndc" named will be asked to shutdown by
+       means of rndc stop.
+       if kill_method=="sigterm" named will be killed by SIGTERM on
+       POSIX systems or by TerminateProcess() on Windows systems.
+
+       :param named_proc: named process instance
+       :type named_proc: subprocess.Popen
+
+       :param resolver: target resolver
+       :type resolver: dns.resolver.Resolver
+
+       :param rndc_cmd: rndc command with default arguments
+       :type rndc_cmd: list of strings, e.g. ["rndc", "-p", "23750"]
+
+       :kill_method: "rndc" or "sigterm"
+       :type kill_method: str
+
+       :param n_workers: Number of worker threads to create
+       :type n_workers: int
+
+       :param n_queries: Total number of queries to send
+       :type n_queries: int
+    """
+# pylint: disable-msg=too-many-arguments
+# pylint: disable-msg=too-many-locals
+
+    # helper function, args must be a list or tuple with arguments to rndc.
+    def launch_rndc(args):
+        return subprocess.call(rndc_cmd + args, timeout=10)
+
+    # We're going to execute queries in parallel by means of a thread pool.
+    # dnspython functions block, so we need to circunvent that.
+    executor = ThreadPoolExecutor(n_workers + 1)
+
+    # Helper dict, where keys=Future objects and values are tags used
+    # to process results later.
+    futures = {}
+
+    # 50% of work will be A queries.
+    # 1 work will be rndc stop.
+    # Remaining work will be rndc status (so we test parallel control
+    #  connections that were crashing named).
+    shutdown = True
+    for i in range(n_queries):
+        if i < (n_queries // 2):
+            # Half work will be standard A queries.
+            # Among those we split 50% queries relname='www',
+            # 50% queries relname=random characters
+            if random.randrange(2) == 1:
+                tag = "good"
+                relname = "www"
+            else:
+                tag = "bad"
+                length = random.randint(4, 10)
+                relname = "".join(letters[
+                    random.randrange(len(letters))] for i in range(length))
+
+            qname = relname + ".test"
+            futures[executor.submit(resolver.query, qname, 'A')] = tag
+        elif shutdown:  # We attempt to stop named in the middle
+            shutdown = False
+            if kill_method == "rndc":
+                futures[executor.submit(launch_rndc, ['stop'])] = 'stop'
+            else:
+                futures[executor.submit(named_proc.terminate)] = 'kill'
+
+        else:
+            # We attempt to send couple rndc commands while named is
+            # being shutdown
+            futures[executor.submit(launch_rndc, ['status'])] = 'status'
+
+    ret_code = -1
+    for future in as_completed(futures):
+        try:
+            result = future.result()
+            # If tag is "stop", result is an instance of
+            # subprocess.CompletedProcess, then we check returncode
+            # attribute to know if rncd stop command finished successfully.
+            #
+            # if tag is "kill" then the main function will check if
+            # named process exited gracefully after SIGTERM signal.
+            if futures[future] == "stop":
+                ret_code = result
+
+        except (dns.resolver.NXDOMAIN, dns.exception.Timeout):
+            pass
+
+    if kill_method == "rndc":
+        assert ret_code == 0
+
+    executor.shutdown()
+
+
+@pytest.mark.dnspython
+def test_named_shutdown(named_port, control_port):
+    # pylint: disable-msg=too-many-locals
+    cfg_dir = os.path.join(os.getcwd(), "resolver")
+    assert os.path.isdir(cfg_dir)
+
+    cfg_file = os.path.join(cfg_dir, "named.conf")
+    assert os.path.isfile(cfg_file)
+
+    named = os.getenv("NAMED")
+    assert named is not None
+
+    rndc = os.getenv("RNDC")
+    assert rndc is not None
+
+    systest_dir = os.getenv("SYSTEMTESTTOP")
+    assert systest_dir is not None
+
+    # rndc configuration resides in $SYSTEMTESTTOP/common/rndc.conf
+    rndc_cfg = os.path.join(systest_dir, "common", "rndc.conf")
+    assert os.path.isfile(rndc_cfg)
+
+    # rndc command with default arguments.
+    rndc_cmd = [rndc, "-c", rndc_cfg, "-p", str(control_port),
+                "-s", "10.53.0.3"]
+
+    # Helper function, launch named without blocking.
+    def launch_named():
+        proc = subprocess.Popen([named, "-c", cfg_file, "-f"], cwd=cfg_dir)
+        # Ensure named is running
+        assert proc.poll() is None
+
+        return proc
+
+    # We create a resolver instance that will be used to send queries.
+    resolver = dns.resolver.Resolver()
+    resolver.nameservers = ['10.53.0.3']
+    resolver.port = named_port
+
+    # We test named shutting down using two methods:
+    # Method 1: using rndc ctop
+    # Method 2: killing with SIGTERM
+    # In both methods named should exit gracefully.
+    for kill_method in ("rndc", "sigterm"):
+        named_proc = launch_named()
+        time.sleep(2)
+
+        do_work(named_proc, resolver, rndc_cmd,
+                kill_method, n_workers=12, n_queries=16)
+
+        # Wait named to exit for a maximum of MAX_TIMEOUT seconds.
+        MAX_TIMEOUT = 10
+        is_dead = False
+        for _ in range(MAX_TIMEOUT):
+            if named_proc.poll() is not None:
+                is_dead = True
+                break
+            time.sleep(1)
+
+        if not is_dead:
+            named_proc.kill()
+
+        assert is_dead
+        # Ensures that named exited gracefully.
+        # If it crashed (abort()) exitcode will be non zero.
+        assert named_proc.returncode == 0

util/copyrights

@@ -784,6 +784,10 @@
 ./bin/tests/system/sfcache/ns5/sign.sh		SH	2018,2019,2020
 ./bin/tests/system/sfcache/setup.sh		SH	2014,2016,2017,2018,2019,2020
 ./bin/tests/system/sfcache/tests.sh		SH	2014,2016,2017,2018,2019,2020
+./bin/tests/system/shutdown/clean.sh		SH	2020
+./bin/tests/system/shutdown/conftest.py		PYTHON	2020
+./bin/tests/system/shutdown/setup.sh		SH	2020
+./bin/tests/system/shutdown/tests-shutdown.py	PYTHON-BIN	2020
 ./bin/tests/system/smartsign/clean.sh		SH	2010,2012,2014,2016,2018,2019,2020
 ./bin/tests/system/smartsign/tests.sh		SH	2010,2011,2012,2014,2016,2017,2018,2019,2020
 ./bin/tests/system/sortlist/clean.sh		SH	2000,2001,2004,2007,2009,2012,2014,2015,2016,2018,2019,2020