upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-10 19:20:00 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Prevent garbage-collecting ignored TCP connections

Browse source 
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
e407888507 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 e407888507, 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.
This commit is contained in:
Michał Kępień 2025-12-21 06:25:56 +01:00
parent 0ec94e501a
commit 1acde358ea
No known key found for this signature in database
1 changed files with 17 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
bin/tests/system/isctest/asyncserver.py
View file

@ -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