fix(core): release sockets when close runs before engine setup completes

bdraco · bdraco · commit 94fc4ec8e16c · 2026-05-17T13:37:15.000-07:00
When ``Zeroconf`` is constructed inside a running asyncio loop, the engine's socket→transport wrapping happens in a scheduled task. If the caller closed the instance before that task ran (e.g. an error path that bails out immediately, or a sync ``zc.close()`` called from the same loop before any await), ``_async_shutdown`` had no transports to close and the raw sockets passed to the engine in ``__init__`` leaked their FDs until interpreter shutdown. ``_async_create_endpoints`` now releases its handle on each socket as the transport adopts it, and ``_async_shutdown`` cancels any pending setup task and closes whatever sockets were never adopted. The async close path tolerates a cancelled setup task instead of asserting on ``_cleanup_timer`` that was never scheduled. Also clean up the test-side leaks surfaced by issue #1133: add the missing ``async_close`` / ``close`` calls in tests that constructed a ``Zeroconf`` or ``AsyncZeroconf`` and never tore it down, give ``test_shutdown_loop`` an explicit ``loop.close()``, and add a no-op ``update_service`` to the ``ServiceListener`` subclass in ``test_service_browser_listeners_no_update_service`` so the browser thread doesn't surface a ``NotImplementedError``. Closes #1133.
diff --git a/src/zeroconf/_engine.py b/src/zeroconf/_engine.py
@@ -23,6 +23,7 @@
 from __future__ import annotations
 
 import asyncio
+import contextlib
 import itertools
 import socket
 import threading
@@ -114,6 +115,13 @@ async def _async_create_endpoints(self) -> None:
                 lambda: AsyncListener(self.zc),  # type: ignore[arg-type, return-value]
                 sock=s,
             )
+            # The transport now owns ``s``; drop our reference so
+            # ``_async_shutdown`` only closes sockets that never reached a
+            # transport (e.g. when setup is cancelled mid-flight).
+            if s is self._listen_socket:
+                self._listen_socket = None
+            if s in self._respond_sockets:
+                self._respond_sockets.remove(s)
             self.protocols.append(cast(AsyncListener, protocol))
             self.readers.append(make_wrapped_transport(cast(asyncio.DatagramTransport, transport)))
             if s in sender_sockets:
@@ -139,19 +147,36 @@ def _async_schedule_next_cache_cleanup(self) -> None:
     async def _async_close(self) -> None:
         """Cancel and wait for the cleanup task to finish."""
         assert self._setup_task is not None
-        await self._setup_task
+        # Setup may have been cancelled by a prior ``_async_shutdown`` (e.g.
+        # close-before-start); swallow the cancellation so close still runs
+        # to completion.
+        with contextlib.suppress(asyncio.CancelledError):
+            await self._setup_task
         self._async_shutdown()
         await asyncio.sleep(0)  # flush out any call soons
-        assert self._cleanup_timer is not None
-        self._cleanup_timer.cancel()
+        if self._cleanup_timer is not None:
+            self._cleanup_timer.cancel()
 
     def _async_shutdown(self) -> None:
         """Shutdown transports and sockets."""
         assert self.running_future is not None
         assert self.loop is not None
         self.running_future = self.loop.create_future()
+        # Stop ``_async_create_endpoints`` from continuing to wrap sockets
+        # after shutdown has started — otherwise it would create transports
+        # that nobody closes, leaking FDs.
+        if self._setup_task is not None and not self._setup_task.done():
+            self._setup_task.cancel()
         for wrapped_transport in itertools.chain(self.senders, self.readers):
             wrapped_transport.transport.close()
+        # Anything still in these collections was never adopted by a
+        # transport; release the FDs so they don't leak.
+        if self._listen_socket is not None:
+            self._listen_socket.close()
+            self._listen_socket = None
+        for s in self._respond_sockets:
+            s.close()
+        self._respond_sockets = []
 
     def close(self) -> None:
         """Close from sync context.
diff --git a/tests/services/test_browser.py b/tests/services/test_browser.py
@@ -1051,6 +1051,9 @@ def remove_service(self, zc, type_, name) -> None:  # type: ignore[no-untyped-de
             if name == registration_name:
                 callbacks.append(("remove", type_, name))
 
+        def update_service(self, zc, type_, name) -> None:  # type: ignore[no-untyped-def]
+            """No-op so the browser thread doesn't see ``NotImplementedError``."""
+
     listener = MyServiceListener()
 
     browser = r.ServiceBrowser(zc, type_, None, listener)
diff --git a/tests/services/test_info.py b/tests/services/test_info.py
@@ -1529,6 +1529,7 @@ async def test_bad_ip_addresses_ignored_in_cache():
     info = ServiceInfo(type_, registration_name)
     info.load_from_cache(aiozc.zeroconf)
     assert info.addresses_by_version(IPVersion.V4Only) == [b"\x7f\x00\x00\x01"]
+    await aiozc.async_close()
 
 
 @pytest.mark.asyncio
@@ -1804,6 +1805,7 @@ async def test_address_resolver():
     aiozc.zeroconf.async_send(outgoing)
     assert await resolve_task
     assert resolver.addresses == [b"\x7f\x00\x00\x01"]
+    await aiozc.async_close()
 
 
 @pytest.mark.asyncio
@@ -1828,6 +1830,7 @@ async def test_address_resolver_ipv4():
     aiozc.zeroconf.async_send(outgoing)
     assert await resolve_task
     assert resolver.addresses == [b"\x7f\x00\x00\x01"]
+    await aiozc.async_close()
 
 
 @pytest.mark.asyncio
@@ -1854,6 +1857,7 @@ async def test_address_resolver_ipv6():
     aiozc.zeroconf.async_send(outgoing)
     assert await resolve_task
     assert resolver.ip_addresses_by_version(IPVersion.All) == [ip_address("fe80::52e:c2f2:bc5f:e9c6")]
+    await aiozc.async_close()
 
 
 @pytest.mark.asyncio
diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py
@@ -500,6 +500,7 @@ async def test_async_service_registration_name_strict_check(quick_timing: None)
 
     await aiozc.async_unregister_service(info)
     await aiozc.async_close()
+    zc.close()
 
 
 @pytest.mark.asyncio
diff --git a/tests/test_core.py b/tests/test_core.py
@@ -824,6 +824,9 @@ def _background_register():
 async def test_event_loop_blocked(mock_start):
     """Test we raise NotRunningException when waiting for startup that times out."""
     aiozc = AsyncZeroconf(interfaces=["127.0.0.1"])
-    with pytest.raises(NotRunningException):
-        await aiozc.zeroconf.async_wait_for_start(timeout=0)
-    assert aiozc.zeroconf.started is False
+    try:
+        with pytest.raises(NotRunningException):
+            await aiozc.zeroconf.async_wait_for_start(timeout=0)
+        assert aiozc.zeroconf.started is False
+    finally:
+        await aiozc.async_close()
diff --git a/tests/test_handlers.py b/tests/test_handlers.py
@@ -1799,6 +1799,8 @@ async def test_response_aggregation_timings_multiple(run_isolated, disable_dupli
         zc.record_manager.async_updates_from_response(incoming)
         assert info2.dns_pointer() in incoming.answers()
 
+    await aiozc.async_close()
+
 
 @pytest.mark.asyncio
 async def test_response_aggregation_random_delay():
diff --git a/tests/utils/test_asyncio.py b/tests/utils/test_asyncio.py
@@ -105,6 +105,7 @@ def _run_coro() -> None:
 
     assert loop.is_running() is False
     runcoro_thread.join()
+    loop.close()
 
 
 def test_cumulative_timeouts_less_than_close_plus_buffer():
