Prevent garbage-collecting ignored TCP connections

Due to the way various asyncio-related objects (tasks, streams,
transports, selectors) are referencing each other, pausing reads for a
TCP transport (which in practice means removing the client socket from
the set of descriptors monitored by a selector) can cause the client
task (AsyncDnsServer._handle_tcp()) to be prematurely garbage-collected,
causing asyncio code to raise a "Task was destroyed but it is pending!"
exception.  Who knew that solutions as elegant as the one introduced by
e4078885073a6c5b59729f4313108e3e7637efdb could cause unexpected trouble?

Fix by making a horrible hack even more horrible, specifically by
keeping a reference to each incoming TCP connection to protect its
related asyncio objects from getting garbage-collected.  This prevents
AsyncDnsServer from closing any of the ignored TCP connections
indefinitely, which is obviously a pretty brain-dead idea for a
production-grade DNS server, but AsyncDnsServer was never meant to be
one and this hack reliably solves the problem at hand.

Only apply this change for the IgnoreAllConnections handler as the
ConnectionReset handler triggers a connection reset immediately after
pausing reads for an incoming TCP connection.

As pointed out in e4078885073a6c5b59729f4313108e3e7637efdb, the proper
solution would require implementing a custom asyncio transport from
scratch and that is still deemed to be too much work for the purpose at
hand.  Let's see how much longer we can limp along with the existing
approach.

diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py
index 6ec3b00f10d41a068bf5d3f449a725cb98a4b51f..d2b22d7c129d2b5fdbad2f269d9d426613a07f2b 100644 (file)
--- a/bin/tests/system/isctest/asyncserver.py
+++ b/bin/tests/system/isctest/asyncserver.py
@@ -20,6 +20,7 @@ from typing import (
     Dict,
     List,
     Optional,
+    Set,
     Tuple,
     Union,
     cast,
@@ -504,10 +505,26 @@ class IgnoreAllConnections(ConnectionHandler):
     client socket, effectively ignoring all incoming connections.
     """
 
+    _connections: Set[asyncio.StreamWriter] = field(default_factory=set)
+
     async def handle(
         self, reader: asyncio.StreamReader, writer: asyncio.StreamWriter, peer: Peer
     ) -> None:
         block_reading(peer, writer)
+        # Due to the way various asyncio-related objects (tasks, streams,
+        # transports, selectors) are referencing each other, pausing reads for
+        # a TCP transport (which in practice means removing the client socket
+        # from the set of descriptors monitored by a selector) can cause the
+        # client task (AsyncDnsServer._handle_tcp()) to be prematurely
+        # garbage-collected, causing asyncio code to raise a "Task was
+        # destroyed but it is pending!" exception.  Prevent that from happening
+        # by keeping a reference to each incoming TCP connection to protect its
+        # related asyncio objects from getting garbage-collected.  This
+        # prevents AsyncDnsServer from closing any of the ignored TCP
+        # connections indefinitely, which is obviously a pretty brain-dead idea
+        # for a production-grade DNS server, but AsyncDnsServer was never meant
+        # to be one and this hack reliably solves the problem at hand.
+        self._connections.add(writer)
 
 
 @dataclass