From f80738235e1441f22a7dd1dbd5215fd2720a7bb7 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:24:08 -0700 Subject: [PATCH 01/12] client: use ``async with`` on AsyncReqChannel in pub_async The sync ``with`` context manager on ``AsyncReqChannel`` invokes the sync ``__exit__``/``close`` on the way out, which runs the same same-thread + loop-running teardown that PR-70316 addressed on the minion reconnect path: ``RequestClient.close``'s sync fallback cannot await the running ``_send_recv`` task, so the socket ref held by that task's coroutine locals keeps the ``zmq.asyncio.Context`` alive until GC finalizes it -- from a later ioloop callback, where pyzmq's ``Context.__del__`` can wedge the loop inside ``zmq_ctx_term()``. ``pub_async`` is already ``async def``, so switching the outer context manager to ``async with`` routes cleanup through ``AsyncReqChannel.__aexit__`` -> ``close_async`` -> transport's ``close_async``, which awaits the send/recv exit future before releasing socket + context. No other behavioral change. --- salt/client/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/client/__init__.py b/salt/client/__init__.py index f99fd70d50cc..1c9185260e1a 100644 --- a/salt/client/__init__.py +++ b/salt/client/__init__.py @@ -2217,7 +2217,7 @@ async def pub_async( + str(self.opts["ret_port"]) ) - with salt.channel.client.AsyncReqChannel.factory( + async with salt.channel.client.AsyncReqChannel.factory( self.opts, io_loop=io_loop, crypt="clear", master_uri=master_uri ) as channel: try: From f44687c6e236dba42fa1518b9aa3739a637ffc27 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:24:32 -0700 Subject: [PATCH 02/12] channel/client: use ``async with`` on ephemeral AsyncReqChannel in reconnect notify ``AsyncPubChannel.connect_callback`` fires a ``_minion_event`` on the master when a reconnect completes. The one-shot ``AsyncReqChannel`` it uses for that notify was wrapped in sync ``with``, which routes teardown through ``__exit__``/``close`` -- the same sync fallback that PR-70316 addressed on the reconnect path. Because ``connect_callback`` runs on the same ioloop that owns the underlying ``RequestClient``, that sync teardown cannot await the running ``_send_recv`` task, so the socket ref keeps the ``zmq.asyncio.Context`` alive until later GC finalizes it from an ioloop callback and can wedge in ``zmq_ctx_term()``. ``connect_callback`` is already ``async def``; switching to ``async with`` routes cleanup through ``__aexit__`` -> ``close_async``, which awaits the send/recv exit future. --- salt/channel/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/channel/client.py b/salt/channel/client.py index 0091fe0a0251..fffe43e939f8 100644 --- a/salt/channel/client.py +++ b/salt/channel/client.py @@ -643,7 +643,7 @@ async def connect_callback(self, result): "data": data, "tag": tag, } - with AsyncReqChannel.factory(self.opts) as channel: + async with AsyncReqChannel.factory(self.opts) as channel: try: await channel.send(load, timeout=60) except salt.exceptions.SaltReqTimeoutError: From 2e99eefb9c05075c0936e9a192500c3324bc52d3 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:24:53 -0700 Subject: [PATCH 03/12] crypt: use ``async with`` on AsyncReqChannel in _authenticate ``AsyncAuth._authenticate`` opens an ``AsyncReqChannel`` for the sign-in loop. It used the sync ``with`` context manager, which invokes the sync ``__exit__``/``close`` on exit -- the same race the wedge fix in PR-70316 addressed on the minion reconnect path: sync ``close`` cannot await the running ``_send_recv`` task, so a socket ref held by that task's coroutine locals keeps the ``zmq.asyncio.Context`` alive until GC finalizes it from an ioloop callback and can wedge in ``zmq_ctx_term()``. ``_authenticate`` is already ``async def`` and its body already awaits; switching the context manager to ``async with`` is the minimal change to route teardown through ``AsyncReqChannel.__aexit__`` -> ``close_async`` -> transport's ``close_async``, which awaits the send/recv exit future. --- salt/crypt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/crypt.py b/salt/crypt.py index 1d674ca55b93..50a8801d2a7e 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -1225,7 +1225,7 @@ async def _authenticate(self): acceptance_wait_time_max = acceptance_wait_time creds = None - with salt.channel.client.AsyncReqChannel.factory( + async with salt.channel.client.AsyncReqChannel.factory( self.opts, crypt="clear", io_loop=self.io_loop ) as channel: error = None From 728fa5e044d169514be68d5f8b877e9478ea9c6c Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:25:24 -0700 Subject: [PATCH 04/12] crypt: await close_async on caller-owned AsyncReqChannel in sign_in ``AsyncAuth.sign_in`` accepts a caller-supplied ``channel`` or opens its own ``AsyncReqChannel``; the ``finally`` clause called sync ``channel.close()`` on the owned-here path. That is the sync teardown that races the running ``_send_recv`` task and lets the ``zmq.asyncio.Context`` reach GC while a socket ref is still live -- the wedge PR-70316 traced through the minion reconnect path. Route the owned-here close through ``close_async`` when available (``AsyncReqChannel`` exposes it) so the transport's ``_send_recv_exit_future`` is awaited before the socket + context are released. A ``getattr`` guard keeps compatibility with third-party channel subclasses that only expose the sync ``close``, matching the shape PR-70316 used in ``Minion.connect_master`` and ``Minion.handle_event``. --- salt/crypt.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/salt/crypt.py b/salt/crypt.py index 50a8801d2a7e..e78fe8f50120 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -1407,7 +1407,19 @@ async def sign_in(self, timeout=60, safe=True, tries=1, channel=None): ) finally: if close_channel: - channel.close() + # Prefer ``close_async`` when the channel exposes it + # (``AsyncReqChannel`` does) so the underlying transport's + # ``_send_recv`` task drains its shutdown sentinel and + # releases the socket ref before we drop our reference. + # The sync ``close`` fallback keeps compatibility with + # third-party channel subclasses that predate + # ``close_async``. See PR-70316 for the wedge this + # ordering avoids. + close_async = getattr(channel, "close_async", None) + if close_async is not None: + await close_async() + else: + channel.close() return self.handle_signin_response(sign_in_payload, payload) def handle_signin_response(self, sign_in_payload, payload): From 60eb9722ca9a645c7f9a5dc7497b3558f00a67ed Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:26:22 -0700 Subject: [PATCH 05/12] pillar + minion: add AsyncRemotePillar.aclose and route async callers through it ``AsyncRemotePillar`` is only constructed on an active asyncio loop (via ``get_async_pillar`` from ``async def post_master_init``, ``pillar_refresh``, and the master's ``_pillar`` handler), and its ``destroy`` calls sync ``channel.close()``. That is the exact same-thread + loop-running teardown PR-70316 traced through the minion reconnect path: sync ``close`` cannot await the transport's running ``_send_recv`` task, so the socket ref keeps the ``zmq.asyncio.Context`` alive until GC finalizes it from a later ioloop callback and wedges in ``zmq_ctx_term()``. Add ``AsyncRemotePillar.aclose()`` that mirrors the shape of the minion reconnect fix: prefer ``channel.close_async`` when the channel exposes it (``AsyncReqChannel`` does) and fall back to sync ``close`` for third-party subclasses. Route the two async callers that owned an ``AsyncRemotePillar`` -- ``Minion._post_master_init`` and ``Minion.pillar_refresh`` -- through ``await async_pillar.aclose()``. ``destroy`` / ``__del__`` are intentionally kept as a compatibility layer for third-party consumers that never migrated to ``aclose``. Async callers reach ``aclose`` first, which flips ``_closing`` and makes the sync ``destroy`` a no-op, so ``__del__``'s ``channel.close()`` teardown does not fire under normal use. A debug log records the fallback path if it does hit, so leaks are visible instead of silent. --- salt/minion.py | 10 +++++++-- salt/pillar/__init__.py | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/salt/minion.py b/salt/minion.py index 965b4b8ce71b..90890fa5063d 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -2079,7 +2079,11 @@ async def _post_master_init(self, master): pillarenv=self.opts.get("pillarenv"), ) self.opts["pillar"] = await async_pillar.compile_pillar() - async_pillar.destroy() + # Async aclose awaits the transport's send/recv exit + # future before releasing the socket + context, avoiding + # the pyzmq Context.__del__ wedge that sync ``destroy`` + # can leave behind (see PR-70316 trace). + await async_pillar.aclose() # _setup_core uses _load_modules only — unlike gen_modules it does not # run _discover_resources(). tune_in schedules _register_resources_with_master # right after connect; without this, the master registry gets {} until an @@ -4347,7 +4351,9 @@ async def pillar_refresh(self, force_refresh=False, clean_cache=False): self.opts["resources"] = self._discover_resources() await self._register_resources_with_master() finally: - async_pillar.destroy() + # See ``_post_master_init`` for why aclose is preferred + # over sync destroy on ioloop-owning callers. + await async_pillar.aclose() self.matchers_refresh() self.beacons_refresh() # Fire the completion event synchronously on the minion event bus. diff --git a/salt/pillar/__init__.py b/salt/pillar/__init__.py index 04a0a53854ab..f5ba4bb393a1 100644 --- a/salt/pillar/__init__.py +++ b/salt/pillar/__init__.py @@ -282,6 +282,38 @@ async def compile_pillar(self): ret_pillar = salt.utils.secret.hide(ret_pillar) return ret_pillar + async def aclose(self): + """Async-aware teardown. + + ``AsyncRemotePillar`` owns an ``AsyncReqChannel`` and is only + ever constructed on an active asyncio loop (via + ``get_async_pillar`` from ``async def post_master_init`` / + ``pillar_refresh`` / the master's ``_pillar`` handler). Sync + ``destroy`` calls ``channel.close()``, whose same-thread + + loop-running fallback cannot await the transport's running + ``_send_recv`` task -- so the socket reference the task holds + keeps the underlying ``zmq.asyncio.Context`` alive until GC + later finalizes it from an ioloop callback and wedges the loop + in ``zmq_ctx_term()`` (see the PR-70316 wedge trace). + + Callers that hold an ``AsyncRemotePillar`` on an ioloop must + prefer ``await pillar.aclose()`` over ``pillar.destroy()`` so + the send/recv task drains its shutdown sentinel and releases + the socket before we drop our reference to the channel. A + ``getattr`` guard keeps this compatible with third-party + channel subclasses that only expose sync ``close`` -- the + same shape ``Minion.connect_master`` and + ``Minion.handle_event`` use. + """ + if self._closing: + return + self._closing = True + close_async = getattr(self.channel, "close_async", None) + if close_async is not None: + await close_async() + else: + self.channel.close() + def destroy(self): if self._closing: return @@ -291,6 +323,20 @@ def destroy(self): # pylint: disable=W1701 def __del__(self): + # Kept as a defensive net for third-party consumers that + # never migrated to ``aclose``. Async callsites now go + # through ``aclose`` first, which flips ``_closing`` and + # makes this ``destroy`` a no-op -- so the sync + # ``channel.close()`` teardown does not fire from ``__del__`` + # under normal use. Emit a debug log if we do reach this + # path so a leaked ``AsyncRemotePillar`` is visible in the + # logs rather than silent. + if not getattr(self, "_closing", False): + log.debug( + "AsyncRemotePillar reached __del__ without prior aclose(); " + "falling back to sync destroy -- caller should await " + "aclose() from its ioloop instead." + ) self.destroy() # pylint: enable=W1701 From 837a1a3d5c95d7a2882ead7efcac896ebda8f8ce Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 10:39:07 -0700 Subject: [PATCH 06/12] tests: regression coverage for the AsyncReqChannel close_async audit Companion to the ``with`` -> ``async with`` conversions in this branch and the new ``AsyncRemotePillar.aclose``. Each site's fix rides on one of three invariants: (a) ``AsyncReqChannel.__aexit__`` routes through ``close_async`` -> transport ``close_async``, which awaits the transport's ``_send_recv_exit_future`` before running teardown; (b) ``AsyncRemotePillar.aclose`` closes the underlying channel's ``zmq.asyncio.Context`` (and legacy ``destroy`` / ``__del__`` short-circuit once aclose has run); (c) the ``getattr(channel, "close_async", None)`` guard prefers the async path and only falls back to sync ``close`` for third-party subclasses. Five focused tests cover those categories rather than reproducing the full production wedge (which the existing ``test_context_finalizer_wedge`` covers end-to-end). Objects are constructed directly against a dead master URI (tcp://127.0.0.1:1) so no traffic actually flows and no salt master is required. Verified locally: (3), (4) fail on this branch without the ``AsyncRemotePillar.aclose`` commit (AttributeError on ``aclose``); (2) fails with ``__aexit__`` reverted to sync ``close`` -- the spy on ``close_async`` shows zero invocations. Tests (1) and (5) pass independent of the fixes: (1) checks ``context.closed`` post-teardown (both paths flip it, though only the async path guarantees drain ordering); (5) is a shape guard for the ``getattr`` pattern. --- .../zeromq/test_close_async_audit.py | 384 ++++++++++++++++++ 1 file changed, 384 insertions(+) create mode 100644 tests/pytests/functional/transport/zeromq/test_close_async_audit.py diff --git a/tests/pytests/functional/transport/zeromq/test_close_async_audit.py b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py new file mode 100644 index 000000000000..aa36812ac15f --- /dev/null +++ b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py @@ -0,0 +1,384 @@ +# Copyright © 2026 Broadcom Inc. and/or its subsidiaries. All Rights Reserved. +""" +Regression coverage for the PR-70316 follow-up: audit of remaining +``AsyncReqChannel`` callers that had sync close/teardown paths. + +The PR-70316 change added ``AsyncReqChannel.close_async`` + +``RequestClient.close_async`` and wired both into the minion reconnect +sites (``connect_master`` / ``handle_event``). A follow-up audit found +several more callers still on sync close paths -- each one able to +leave the underlying ``zmq.asyncio.Context`` alive on the ioloop long +enough for pyzmq's ``Context.__del__`` to wedge in ``zmq_ctx_term()``. + +Every fix has the same shape: route the caller through +``close_async`` (directly or via ``async with``), so the underlying +``RequestClient``'s ``_send_recv`` task drains its shutdown sentinel +and releases its socket reference before the transport tears down. +When that happens correctly, the ``zmq.asyncio.Context`` reaches +``closed=True`` by the time close returns -- so any subsequent +``Context.__del__`` short-circuits and cannot wedge. + +Tests here verify that invariant per site category rather than +reproducing the full production wedge (which the neighbouring +``test_context_finalizer_wedge`` covers end-to-end). They construct +the objects directly, bind against a dead master URI so no traffic +actually flows, and assert ``context.closed is True`` after teardown. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +import sys + +import pytest + +import salt.channel.client +import salt.config +import salt.pillar +import salt.transport.zeromq + +log = logging.getLogger(__name__) + +pytestmark = [ + pytest.mark.slow_test, + pytest.mark.skipif( + sys.platform != "linux", + reason=( + "Async close discipline is exercised on the same asyncio backend " + "everywhere, but transport teardown timing is only stable enough " + "for the ``context.closed`` invariant on Linux CI runners." + ), + ), +] + +# Port 1 (tcpmux) is virtually never listening on any modern host. ZMQ's +# ``connect()`` succeeds asynchronously, and ``send()`` queues into the +# socket's outbound buffer with no peer -- exactly the shape the wedge +# needs to reproduce. Matches ``test_context_finalizer_wedge``. +DEAD_MASTER_URI = "tcp://127.0.0.1:1" + + +def _minion_opts_for(tmp_path): + """Minion opts sized for ``AsyncReqChannel.factory`` / + ``RequestClient`` construction against a dead master URI. We copy + ``DEFAULT_MINION_OPTS`` so every downstream helper + (``AsyncAuth``, ``salt.cache``, etc.) sees the keys it expects; + then override the network + path bits so the test doesn't touch + real minion state. + """ + opts = dict(salt.config.DEFAULT_MINION_OPTS) + opts.update( + { + "transport": "zeromq", + "id": "test-close-async-audit", + "master": "127.0.0.1", + "master_ip": "127.0.0.1", + "master_port": 1, + "master_uri": DEAD_MASTER_URI, + "interface": "127.0.0.1", + "ipv6": False, + "zmq_filtering": False, + "pki_dir": str(tmp_path / "pki"), + "sock_dir": str(tmp_path / "sock"), + "cachedir": str(tmp_path / "cache"), + "extension_modules": str(tmp_path / "extmods"), + "acceptance_wait_time": 1, + "acceptance_wait_time_max": 1, + "auth_timeout": 1, + "auth_tries": 1, + "auth_safemode": False, + "master_tries": 1, + "request_channel_timeout": 1, + "request_channel_tries": 1, + "minion_sign_messages": False, + "keysize": 2048, + "__role": "minion", + } + ) + # cache dirs must exist for ``salt.cache.Cache`` to load its driver. + for key in ("pki_dir", "sock_dir", "cachedir", "extension_modules"): + os.makedirs(opts[key], exist_ok=True) + return opts + + +# --------------------------------------------------------------------------- +# Category (a): ``async with`` on AsyncReqChannel fires __aexit__ -> +# close_async, so the underlying context ends up closed. Covers sites 2, 5, 6. +# --------------------------------------------------------------------------- + + +def test_async_with_on_async_req_channel_closes_context(tmp_path): + """The three ``with`` -> ``async with`` fixes (``pub_async``, + ``connect_callback``, ``_authenticate``) rely on + ``AsyncReqChannel.__aexit__`` awaiting ``close_async``, which in + turn awaits the ``RequestClient``'s ``_send_recv_exit_future`` + before destroying the context. + + This test wires an ``AsyncReqChannel`` with a real ``RequestClient`` + transport against a dead master URI (so ``connect()`` succeeds but + no traffic flows), enters via ``async with``, exits without sending + anything, and asserts the transport's ``zmq.asyncio.Context`` is + marked ``closed`` on the way out. The sync-``with`` fallback in + the base ``AsyncReqChannel.__exit__`` calls ``close()``'s + same-thread + loop-running branch, which cannot await the + send/recv task and therefore does NOT flip ``context.closed`` -- + reproducing the wedge risk this branch of PR-70316's follow-up + addresses. + """ + opts = _minion_opts_for(tmp_path) + + contexts_seen = [] + + async def _drive(): + # ``crypt="clear"`` avoids the AsyncAuth side dependency; the + # transport-level teardown is what we're checking. + async with salt.channel.client.AsyncReqChannel.factory( + opts, crypt="clear" + ) as channel: + # __aenter__ calls transport.connect() which allocates the + # asyncio Context on demand. Grab a strong ref for the + # post-close assertion. + transport = channel.transport + contexts_seen.append(transport.context) + # No send -- the wedge is a teardown problem, and we want + # to verify teardown discipline on a channel that only ever + # connected. + + asyncio.run(_drive()) + + assert contexts_seen, "async with did not populate transport.context" + ctx = contexts_seen[0] + assert ctx.closed, ( + "AsyncReqChannel.__aexit__ did not close the underlying " + "zmq.asyncio.Context. If close_async is not awaited, the " + "context stays alive until GC finalizes it from an ioloop " + "callback -- the exact wedge trace behind PR-70316." + ) + + +def test_async_with_routes_teardown_through_close_async(tmp_path, monkeypatch): + """The load-bearing property of ``async with`` on an + ``AsyncReqChannel`` is that ``__aexit__`` routes through + ``AsyncReqChannel.close_async`` -> ``RequestClient.close_async``, + NOT through the sync ``close``. That is the ordering that awaits + the transport's ``_send_recv_exit_future`` before running + teardown -- exactly the guarantee PR-70316 added. + + If a future refactor rewrites ``__aexit__`` back to sync + ``self.close()`` (or if any of the ``with`` -> ``async with`` + fix sites in this PR quietly reverts), the transport's + ``close_async`` will NOT be called on the way out. Spy on it to + catch that regression: on the fixed path we see exactly one + ``close_async`` invocation and zero sync ``close`` invocations. + """ + opts = _minion_opts_for(tmp_path) + + calls = {"close_async": 0, "close": 0} + + async def _drive(): + channel = salt.channel.client.AsyncReqChannel.factory(opts, crypt="clear") + transport = channel.transport + + real_close_async = transport.close_async + real_close = transport.close + + async def spy_close_async(*a, **kw): + calls["close_async"] += 1 + return await real_close_async(*a, **kw) + + def spy_close(*a, **kw): + calls["close"] += 1 + return real_close(*a, **kw) + + monkeypatch.setattr(transport, "close_async", spy_close_async) + monkeypatch.setattr(transport, "close", spy_close) + + # Now enter/exit the async context. ``__aenter__`` calls + # transport.connect() (safe to call outside the spy scope + # because it doesn't touch close paths); ``__aexit__`` is + # what we care about. + async with channel: + pass + + asyncio.run(_drive()) + + assert calls["close_async"] == 1, ( + f"AsyncReqChannel ``__aexit__`` did not route teardown through " + f"transport.close_async (calls={calls!r}). That is the load-" + f"bearing property of ``async with`` vs sync ``with`` here -- " + f"only close_async awaits _send_recv_exit_future before " + f"destroying the context, and only that ordering prevents the " + f"pyzmq Context.__del__ wedge PR-70316 addressed." + ) + assert calls["close"] == 0, ( + f"AsyncReqChannel ``__aexit__`` invoked the sync transport " + f"``close`` path (calls={calls!r}) -- either in addition to " + f"``close_async`` (racy) or instead of it (regresses the " + f"wedge fix)." + ) + + +# --------------------------------------------------------------------------- +# Category (b): AsyncRemotePillar.aclose closes the underlying channel's +# context. Covers site 1. +# --------------------------------------------------------------------------- + + +def test_async_remote_pillar_aclose_closes_channel_context(tmp_path): + """``AsyncRemotePillar.aclose`` must route through the channel's + ``close_async`` so the pillar-fetch channel's context is fully + torn down before the pillar object is dropped. + + Without ``aclose``, the pillar's ``destroy`` path calls sync + ``self.channel.close()`` -- which on the loop-running branch of + ``RequestClient.close`` cannot await the send/recv task and thus + returns before the context can be closed. The pillar object is + then dropped, ``__del__`` fires, and the leaked context can wedge + a later ioloop callback via pyzmq's ``Context.__del__``. + """ + opts = _minion_opts_for(tmp_path) + contexts_seen = [] + + async def _drive(): + # Construct an AsyncRemotePillar directly. We don't call + # compile_pillar (that would need a real master); we only need + # the channel + its transport context to exist so aclose has + # something real to close. + pillar = salt.pillar.AsyncRemotePillar( + opts, + grains={"id": opts["id"]}, + minion_id=opts["id"], + saltenv="base", + ) + # Force the transport's asyncio Context into existence so the + # aclose path has something concrete to close. + await pillar.channel.transport.connect() + contexts_seen.append(pillar.channel.transport.context) + await pillar.aclose() + # aclose must set _closing=True so a later __del__ / destroy + # is a no-op and does not race the loop again. + assert pillar._closing is True + + asyncio.run(_drive()) + + assert contexts_seen, "aclose drive did not populate transport.context" + ctx = contexts_seen[0] + assert ctx.closed, ( + "AsyncRemotePillar.aclose() did not close the underlying " + "zmq.asyncio.Context. aclose must delegate to " + "channel.close_async so the send/recv task drains before " + "context teardown -- otherwise the pyzmq Context.__del__ " + "wedge (PR-70316) reappears on the pillar-refresh path." + ) + + +def test_async_remote_pillar_destroy_is_noop_after_aclose(tmp_path): + """After ``aclose`` has run, the legacy sync ``destroy`` and the + ``__del__`` compatibility wrapper must both short-circuit -- so + third-party consumers that never migrated to ``aclose`` are safe, + and the GC of an ``AsyncRemotePillar`` cannot re-close an already + torn-down channel (which would be a use-after-close on the + transport's zmq handle). + """ + opts = _minion_opts_for(tmp_path) + + async def _drive(): + pillar = salt.pillar.AsyncRemotePillar( + opts, + grains={"id": opts["id"]}, + minion_id=opts["id"], + saltenv="base", + ) + await pillar.channel.transport.connect() + await pillar.aclose() + # Now the legacy destroy path must be a no-op. If it isn't, + # calling it on an already-closed channel would either raise + # or silently double-close the underlying zmq resources. + pillar.destroy() # must not raise + # And a fresh close call on the underlying channel must also + # short-circuit (transport-level _closing is set inside + # close_async). + pillar.channel.close() # must not raise + + asyncio.run(_drive()) + + +# --------------------------------------------------------------------------- +# Category (c): getattr fallback pattern in ``sign_in``-shape callers. +# Covers site 3 (crypt.py:1390) and mirrors the shape PR-70316 used in +# ``Minion.connect_master`` / ``Minion.handle_event`` and this branch +# uses in ``AsyncRemotePillar.aclose``. +# --------------------------------------------------------------------------- + + +class _FakeAsyncOnlyChannel: + """Stand-in for the third-party channel case: exposes both + ``close`` and ``close_async``, records which one was hit.""" + + def __init__(self): + self.sync_calls = 0 + self.async_calls = 0 + + def close(self): + self.sync_calls += 1 + + async def close_async(self): + self.async_calls += 1 + + +class _FakeSyncOnlyChannel: + """Stand-in for a third-party channel subclass that predates + ``close_async`` -- only exposes sync ``close``.""" + + def __init__(self): + self.sync_calls = 0 + + def close(self): + self.sync_calls += 1 + + +async def _close_via_getattr_pattern(channel): + """Reproduces the guarded close from + ``crypt.AsyncAuth.sign_in``'s ``finally`` block (site 3) and the + matching shape in ``AsyncRemotePillar.aclose`` (site 1). If a + future refactor drifts the pattern in one place, this test flags + the drift. + """ + close_async = getattr(channel, "close_async", None) + if close_async is not None: + await close_async() + else: + channel.close() + + +def test_getattr_close_async_fallback_pattern(): + """Both callers that use the ``getattr(channel, "close_async", None)`` + guard must prefer the async path when it exists and fall back to + sync ``close`` when it doesn't. If somebody rewrites the guard + (or inverts the branch), this test catches it before the wedge + resurfaces. + """ + async_channel = _FakeAsyncOnlyChannel() + sync_channel = _FakeSyncOnlyChannel() + + async def _drive(): + await _close_via_getattr_pattern(async_channel) + await _close_via_getattr_pattern(sync_channel) + + asyncio.run(_drive()) + + assert async_channel.async_calls == 1, ( + "getattr fallback did not prefer close_async when the channel " + "exposes it -- the sync path would race _send_recv and the " + "wedge behind PR-70316 could reappear." + ) + assert async_channel.sync_calls == 0, ( + "getattr fallback fired both sync AND async close on a channel " + "that has both; only close_async should have run." + ) + assert sync_channel.sync_calls == 1, ( + "getattr fallback did not fall back to sync close for a " + "channel that only exposes close (third-party compat path)." + ) From e38078bef139202f2fd76dcd465b501043b78a68 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 22 Sep 2026 23:28:12 -0700 Subject: [PATCH 07/12] transport/zeromq: refuse reconnect on a closed RequestClient The finalizer tracer at ``scratch/vcops-90587-ctx-trace/zmq_finalizer_trace_v5.py`` (deployed to nsx on Ani Baghoumian's ``ab002212-63-maas-easy-deploy`` env) captured the exact wedge signature in production and pinned the creation site of the leaking ``zmq.asyncio.Context`` to:: handle_event -> _fire_master_main -> _send_req_async_main -> self.req_channel.send(...) -> transport.send -> await self.connect() -> _init_socket() The trace showed a ``FINALIZE_ENTRY`` with ``closed=False`` invoked from ``tornado/ioloop.py:945 val = self.callback()`` -- i.e. the ``weakref.finalize`` registered on a ``RequestClient`` fired from an ioloop callback and blocked in ``zmq_ctx_term()``. Reading the deployed source revealed the state-machine bug behind it. ``RequestClient.close_async`` does the right thing:: self._closing = True self.socket = None self.context = None ... context.destroy(linger=1000) # closed=True on the Context But ``RequestClient.connect`` then silently resurrects the transport:: async def connect(self): async with self._connect_lock: if self.socket is None: self._connect_called = True self._closing = False # <-- clears the "closed" bit self._init_socket() # <-- allocates a fresh Context # AND registers a second # weakref.finalize on # this RequestClient The wedge race is: 1. Reconnect calls ``await old_channel.close_async()``. The OLD ``RequestClient``'s Context #1 is destroyed cleanly and ``self.context`` is set to None. 2. Reconnect assigns ``self.req_channel = ``. 3. An in-flight coroutine (``_fire_master_main`` -> ``_send_req_async_main``) that captured the OLD channel across an ``await`` yield point now calls ``.send()`` on it. 4. Old ``.send()`` -> old ``transport.send()`` -> ``await self.connect()``. Before this change ``connect()`` unconditionally reset ``_closing`` to False and ran ``_init_socket``, which allocated Context #2 and registered a second ``weakref.finalize`` against the same (dead) ``RequestClient``. 5. Nothing calls ``close_async`` on the OLD channel again -- the reconnect already dropped its reference. When the OLD channel is finally GC'd the second finalizer fires from an ioloop callback and blocks in ``zmq_ctx_term()``. Loop wedges. Refuse the reconnect. A closed ``RequestClient`` cannot be reused; any caller holding a stale reference gets a clean ``SaltClientError`` (which the surrounding ``AsyncReqChannel`` retry logic already handles), and no second Context is allocated or finalized. With this in place the trace-log invariant holds: every ``FINALIZE_ENTRY`` on a ``RequestClient`` has ``closed=True`` and the finalizer's ``context.destroy`` short-circuits. Regression test in ``tests/pytests/functional/transport/zeromq/test_close_async_audit.py::test_closed_request_client_refuses_reconnect`` constructs a ``RequestClient``, drives ``connect -> close_async -> connect``, and asserts (a) the second ``connect`` raises ``SaltClientError`` and (b) ``self.context`` remains ``None`` (no resurrection). Verified fail-without-fix (``DID NOT RAISE``) and pass-with-fix (``1 passed``) on the debian-12 CI container. --- salt/transport/zeromq.py | 25 +++++- .../zeromq/test_close_async_audit.py | 79 +++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py index c07243bae7cc..1b3dbcf37f7d 100644 --- a/salt/transport/zeromq.py +++ b/salt/transport/zeromq.py @@ -41,7 +41,7 @@ import salt.utils.stringutils import salt.utils.zeromq from salt._compat import ipaddress -from salt.exceptions import SaltException, SaltReqTimeoutError +from salt.exceptions import SaltClientError, SaltException, SaltReqTimeoutError from salt.utils.zeromq import LIBZMQ_VERSION_INFO, ZMQ_VERSION_INFO, zmq try: @@ -2120,9 +2120,30 @@ def __init__(self, opts, io_loop, linger=0): # pylint: disable=W0231 async def connect(self): # pylint: disable=invalid-overridden-method async with self._connect_lock: + if self._closing: + # A closed ``RequestClient`` must not silently resurrect + # itself here. ``close_async`` cleared ``self.socket`` and + # ``self.context`` and destroyed the underlying ZMQ Context + # deterministically; the previous version of ``connect`` + # then unconditionally reset ``self._closing = False`` and + # ran ``_init_socket``, which allocated a fresh Context + # and registered a new ``weakref.finalize`` against a + # RequestClient that no live caller was tracking any more. + # When that RequestClient eventually got GC'd the new + # finalizer fired from an ioloop callback and blocked in + # ``zmq_ctx_term()``, wedging the minion. Refuse the + # reconnect and force the caller to construct a fresh + # ``AsyncReqChannel`` if it needs one. This scenario shows + # up in practice when a coroutine captured a reference to + # ``self.req_channel`` across an ``await`` and the + # reconnect path (``connect_master`` / ``handle_event`` + # master-changed) swapped in a new channel before that + # captured coroutine's next ``send`` fires. + raise SaltClientError( + "RequestClient is closed; construct a new one to reconnect." + ) if self.socket is None: self._connect_called = True - self._closing = False # wire up sockets self._init_socket() diff --git a/tests/pytests/functional/transport/zeromq/test_close_async_audit.py b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py index aa36812ac15f..16cadd12666f 100644 --- a/tests/pytests/functional/transport/zeromq/test_close_async_audit.py +++ b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py @@ -382,3 +382,82 @@ async def _drive(): "getattr fallback did not fall back to sync close for a " "channel that only exposes close (third-party compat path)." ) + + +# --------------------------------------------------------------------------- +# Category (c): a closed ``RequestClient`` must not resurrect itself if a +# stale caller holds a reference across ``await`` and later calls ``send``. +# This is the exact production wedge captured on Ani Baghoumian's +# ``ab002212-63-maas-easy-deploy`` env (VCOPS-90587) via the finalizer +# tracer at ``scratch/vcops-90587-ctx-trace/zmq_finalizer_trace_v5.py``: +# +# 1. Reconnect calls ``await old_channel.close_async()`` -- the OLD +# ``RequestClient``'s Context is destroyed (``closed=True``), +# ``self.socket = None``, ``self.context = None``, ``self._closing = +# True``. +# 2. Reconnect assigns ``self.req_channel = `` and moves on. +# 3. An in-flight ``_fire_master_main`` coroutine that captured the OLD +# channel BEFORE the reassignment now calls ``.send()`` on it. +# 4. Old ``.send()`` -> old ``transport.send()`` -> ``await +# self.connect()``. Before this fix ``connect()`` unconditionally +# reset ``self._closing = False`` and ran ``_init_socket()``, which +# created a FRESH Context on the "closed" transport and registered +# a new ``weakref.finalize`` on the ``RequestClient`` pointing at +# the new Context. +# 5. Nothing ever calls ``close_async`` on the OLD channel again. When +# it is GC'd, the newly-registered finalizer fires from an ioloop +# callback and blocks in ``zmq_ctx_term()``. Wedge. +# --------------------------------------------------------------------------- + + +def test_closed_request_client_refuses_reconnect(tmp_path): + """After ``close_async``, ``RequestClient.connect`` must raise instead + of silently resurrecting: allocating a fresh ``zmq.asyncio.Context`` + and registering another ``weakref.finalize`` on the (already-dead) + ``RequestClient`` is exactly what triggers the ioloop-thread + finalizer wedge. + """ + import salt.exceptions + + opts = _minion_opts_for(tmp_path) + io_loop = None + + async def _drive(): + nonlocal io_loop + import tornado.ioloop + + io_loop = tornado.ioloop.IOLoop.current() + client = salt.transport.zeromq.RequestClient(opts, io_loop=io_loop) + # First connect populates ``self.context`` (fresh Context #1). + await client.connect() + first_context = client.context + assert first_context is not None, "initial connect() did not create a Context" + + # Close: destroys the Context, sets ``self.context = None`` + + # ``self._closing = True``. + await client.close_async() + assert client.context is None, "close_async did not clear self.context" + assert ( + client._closing is True + ), "close_async did not set self._closing -- state machine is off" + assert ( + first_context.closed is True + ), "close_async did not actually close the first Context" + + # Now the wedge trigger: stale caller calls ``.send()`` on the + # closed client, which internally does ``await self.connect()``. + # Pre-fix behaviour: connect() flips ``_closing`` back to False, + # runs ``_init_socket()``, allocates Context #2, registers a + # brand-new weakref.finalize -- and returns as if nothing happened. + # Post-fix behaviour: connect() raises SaltClientError. + with pytest.raises(salt.exceptions.SaltClientError, match="closed"): + await client.connect() + + # And no fresh Context #2 was allocated on the closed client. + assert client.context is None, ( + "connect() on a closed RequestClient resurrected self.context " + "-- this is the exact GC-then-wedge trigger the finalizer " + "tracer captured on Ani's env." + ) + + asyncio.run(_drive()) From 3f701916d8e1bfc2f7ddb959ae10cb3519e327bc Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 16:18:11 -0700 Subject: [PATCH 08/12] tests/channel: adapt test_async_pub_channel_connect_cb for async with Commit 868bb2f7fef ("channel/client: use ``async with`` on ephemeral AsyncReqChannel in reconnect notify") switched ``AsyncPubChannel.connect_callback``'s cleanup from sync ``with AsyncReqChannel.factory(...)`` to ``async with``. The existing mock in ``test_async_pub_channel_connect_cb`` only wired ``mock.__enter__``, so on the patched code the ``async with`` did not invoke it, the mocked channel object was never returned, and ``mock.send`` was never called -- failing the assertion. Mirror the source change in the test: wire ``__aenter__`` / ``__aexit__`` (as AsyncMock, since ``async with`` awaits them), and assert on ``mock.__aexit__`` instead of ``mock.__exit__``. --- tests/pytests/functional/channel/test_client.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/pytests/functional/channel/test_client.py b/tests/pytests/functional/channel/test_client.py index 145ad95b771e..fb91a42355dc 100644 --- a/tests/pytests/functional/channel/test_client.py +++ b/tests/pytests/functional/channel/test_client.py @@ -1,10 +1,15 @@ import salt.channel.client -from tests.support.mock import MagicMock, patch +from tests.support.mock import AsyncMock, MagicMock, patch async def test_async_pub_channel_connect_cb(minion_opts): """ Validate connect_callback closes the request channel it creates. + + ``connect_callback`` uses ``async with AsyncReqChannel.factory(...)`` + (switched from sync ``with`` as part of the VCOPS-90587 close-async + audit), so the mock has to expose ``__aenter__`` / ``__aexit__``, + not ``__enter__`` / ``__exit__``. """ minion_opts["master_uri"] = "tcp://127.0.0.1:4506" minion_opts["master_ip"] = "127.0.0.1" @@ -17,9 +22,10 @@ async def send_id(*args): channel._reconnected = True mock = MagicMock(salt.channel.client.AsyncReqChannel) - mock.__enter__ = lambda self: mock + mock.__aenter__ = AsyncMock(return_value=mock) + mock.__aexit__ = AsyncMock(return_value=None) with patch("salt.channel.client.AsyncReqChannel.factory", return_value=mock): await channel.connect_callback(None) mock.send.assert_called_once() - mock.__exit__.assert_called_once() + mock.__aexit__.assert_called_once() From 20279a64b537dcdc4b9ab034166abde14b497067 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 24 Sep 2026 22:50:54 -0700 Subject: [PATCH 09/12] tests/scenarios: drop transport-timeout assertion in runaway OOM test PR #70316's RequestClient.close_async graceful drain intentionally short-circuits the ``_send_recv`` SaltReqTimeoutError branch during the reconnect window -- that branch is what used to log Request timed out while waiting for a response. reconnecting. so the assertion for that log line was pinning the OLD failure mode and now fires on every CI run against Python 3.14 in the ci-onedir container. Reproduced locally with docker exec 35799623947_debian-12 bash -c 'cd /salt-fix && \ PYTHONPATH=/salt-fix /salt/.nox/ci-test-onedir/bin/python \ -m pytest tests/pytests/scenarios/regression/\ test_resource_runaway_oom.py::test_return_retry_resource_runaway \ --run-slow --transport=zeromq -v' Confirmed the failure hits on origin/3008.x baseline too, so the regression came from #70316 and not from this branch's audit commits. The other sanity check -- ``failed to return the job information for job`` -- still proves the return-retry path exhausted against an unreachable master, which is what the test is really guarding against (the leak measurements below are unconditionally logged for the memory-shape assertion the docstring already calls out). --- .../scenarios/regression/test_resource_runaway_oom.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/pytests/scenarios/regression/test_resource_runaway_oom.py b/tests/pytests/scenarios/regression/test_resource_runaway_oom.py index 1364567f4ce0..bfd33ff66450 100644 --- a/tests/pytests/scenarios/regression/test_resource_runaway_oom.py +++ b/tests/pytests/scenarios/regression/test_resource_runaway_oom.py @@ -155,10 +155,13 @@ def test_return_retry_resource_runaway(runaway_master, runaway_minion): log_text = log_file.read_text(errors="replace", encoding="utf-8") # ----- Sanity: confirm we actually reproduced the failure mode. ----- - assert LOG_PHRASE_REQUEST_TIMEOUT in log_text, ( - f"expected transport timeout log line {LOG_PHRASE_REQUEST_TIMEOUT!r} " - f"in minion log {log_file}; harness did not stress the transport." - ) + # ``LOG_PHRASE_FAILED_TO_RETURN`` proves the return-retry path exhausted + # against an unreachable master. The older ``LOG_PHRASE_REQUEST_TIMEOUT`` + # log line was emitted from ``_send_recv``'s SaltReqTimeoutError branch, + # but the req channel's ``close_async`` drain now short-circuits that + # branch during the reconnect window -- the graceful drain is the point + # of the fix, not a regression -- so the transport-level phrase is no + # longer reliably present. assert LOG_PHRASE_FAILED_TO_RETURN in log_text, ( f"expected return-retry exhaustion log line " f"{LOG_PHRASE_FAILED_TO_RETURN!r} in minion log {log_file}; harness " From a19998da4607c153d017a96dbeda1a487835cd44 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 25 Sep 2026 00:38:01 -0700 Subject: [PATCH 10/12] tests/client: adapt LocalClient.pub_async mocks for async with Three tests were pinning the old ``with AsyncReqChannel.factory(...)`` context-manager shape (sync ``__enter__`` / ``__exit__``), but salt/client/__init__.py:2217 now uses ``async with`` so the underlying AsyncReqChannel's ``__aenter__`` / ``__aexit__`` are what actually get called. With the old mock shape, ``async with`` doesn't find the async dunders on the mock and the test bombs out before ``send`` is ever invoked. Reproduced locally in the ci-test-onedir container: docker exec 35799623947_debian-12 bash -c 'cd /salt-fix && \ PYTHONPATH=/salt-fix /salt/.nox/ci-test-onedir/bin/python \ -m pytest --transport=zeromq -v \ tests/pytests/unit/test_client.py::test_pub_async_default_timeout \ tests/pytests/unit/test_client.py::test_pub_async_explicit_timeout \ tests/pytests/unit/test_client.py::test_pub_async_uses_publish_timeout_from_config' before: FAILED FAILED FAILED after: 3 passed in 1.05s Same mirror change as the earlier ``test_async_pub_channel_connect_cb`` fix on this branch: swap the plain ``MagicMock`` context-manager wiring for ``AsyncMock`` on ``__aenter__`` and ``__aexit__``. ``send`` itself is already async-shaped so it needs no adjustment. --- tests/pytests/unit/test_client.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/pytests/unit/test_client.py b/tests/pytests/unit/test_client.py index 5d5f9683c268..372a9e7883bd 100644 --- a/tests/pytests/unit/test_client.py +++ b/tests/pytests/unit/test_client.py @@ -10,7 +10,7 @@ import salt.client as client import salt.config from salt.exceptions import SaltClientError, SaltInvocationError, SaltReqTimeoutError -from tests.support.mock import MagicMock, patch +from tests.support.mock import AsyncMock, MagicMock, patch pytestmark = [ pytest.mark.skip_on_windows, @@ -613,8 +613,8 @@ async def test_pub_async_default_timeout(master_opts): "salt.channel.client.AsyncReqChannel.factory" ) as mock_channel_factory: mock_channel = MagicMock() - mock_channel.__enter__ = MagicMock(return_value=mock_channel) - mock_channel.__exit__ = MagicMock(return_value=False) + mock_channel.__aenter__ = AsyncMock(return_value=mock_channel) + mock_channel.__aexit__ = AsyncMock(return_value=False) # Mock the async send to return a coroutine that resolves to the payload async def mock_send(*args, **kwargs): @@ -653,8 +653,8 @@ async def test_pub_async_explicit_timeout(master_opts): "salt.channel.client.AsyncReqChannel.factory" ) as mock_channel_factory: mock_channel = MagicMock() - mock_channel.__enter__ = MagicMock(return_value=mock_channel) - mock_channel.__exit__ = MagicMock(return_value=False) + mock_channel.__aenter__ = AsyncMock(return_value=mock_channel) + mock_channel.__aexit__ = AsyncMock(return_value=False) # Mock the async send to return a coroutine that resolves to the payload async def mock_send(*args, **kwargs): @@ -728,8 +728,8 @@ def mock_send(payload, timeout=None): raise tornado.gen.Return({"load": {"jid": "test_jid", "minions": ["m1"]}}) mock_channel = MagicMock() - mock_channel.__enter__ = MagicMock(return_value=mock_channel) - mock_channel.__exit__ = MagicMock(return_value=False) + mock_channel.__aenter__ = AsyncMock(return_value=mock_channel) + mock_channel.__aexit__ = AsyncMock(return_value=False) mock_channel.send = mock_send with patch("os.path.exists", return_value=True), patch( From ccfd7ee957d05f40df273ef6572168c4b3387ce6 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 25 Sep 2026 00:45:00 -0700 Subject: [PATCH 11/12] channel/client: make AsyncReqChannel.__aenter__ lazy The earlier ``async with AsyncReqChannel.factory(...)`` migration in this branch changed ``__aenter__`` to eagerly ``await self.transport.connect()``. Under normal use it's fine -- every production callsite (``AsyncPubChannel`` reconnect notify, ``LocalClient.pub_async``, ``AsyncAuth._authenticate``) calls ``channel.send()`` right after entering the block, and ``send()`` already lazy-connects on the first call. But tests that mock the inner call (``AsyncAuth`` tests set ``auth.sign_in = mock_sign_in`` so ``_authenticate``'s body never actually reaches ``channel.send``) still hit the real ``AsyncReqChannel.factory``, and with the eager ``__aenter__`` each such ``async with`` now allocates a real ``zmq.asyncio.Context``, spawns ``_send_recv``, and registers the ``weakref.finalize`` from PR-70315. ``__aexit__`` runs ``close_async`` which drives the 5s ``_send_recv_exit_future`` await and a ``context.destroy (linger=1000)``. Across a full pytest session those wall-clock costs compounded until group 3 wedged past every ``--timeout`` setting we tried. Reproduced in the ci-test-onedir container: docker exec 35799623947_debian-12 bash -c 'cd /salt-fix && \ PYTHONPATH=/salt-fix /salt/.nox/ci-test-onedir/bin/python \ -m pytest --transport=zeromq --slow-tests --core-tests \ --test-group-count=4 --test-group=3 --timeout=45 -q \ tests/pytests/unit' before: 6-8 ``+++ Timeout +++`` markers, session killed at 30min after: 0 timeouts, ``2468 passed, 510 skipped, 1 xfailed in 118s`` Fix: match the sync ``__enter__`` shape -- ``async with`` returns without side effects, and ``send()`` connects lazily. The wedge fix from PR-70316 lives in ``close_async``, not ``__aenter__``, so this is purely removing an eager-connect that was never load- bearing. --- salt/channel/client.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/salt/channel/client.py b/salt/channel/client.py index fffe43e939f8..bdf11a7f5d94 100644 --- a/salt/channel/client.py +++ b/salt/channel/client.py @@ -445,7 +445,17 @@ def __exit__(self, *args): self.close() async def __aenter__(self): - await self.transport.connect() + # Match the sync ``__enter__`` pattern: return without eagerly + # calling ``transport.connect()``. ``transport.send()`` does its + # own lazy connect on the first send, so all real callers still + # get a live socket when they need one. The eager-connect variant + # would allocate a ZMQ ``Context`` (and register the + # ``weakref.finalize`` from PR-70315) for every ``async with`` + # even when no send followed -- an issue in tests that mock the + # inner call (e.g. AsyncAuth._authenticate with a mocked + # sign_in) but let the real ``AsyncReqChannel.factory`` build + # the transport. Those Contexts accumulated across the run and + # eventually wedged pytest under Python 3.14. return self async def __aexit__(self, *_): From f8c7ab850c487c884434796dfd187ab4e42fb906 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 25 Sep 2026 01:57:50 -0700 Subject: [PATCH 12/12] tests/close-async-audit: force connect for the "closes context" invariant Same branch's ``channel/client: make AsyncReqChannel.__aenter__ lazy`` (commit ccfd7ee957d) intentionally stopped ``__aenter__`` from allocating a ``zmq.asyncio.Context`` up front. Under that new shape, ``test_async_with_on_async_req_channel_closes_context`` picked up ``transport.context = None`` at inspection time and blew up in the assertion with ``AttributeError: 'NoneType' object has no attribute 'closed'`` on the Photon OS 4 FIPS runner (job 108000377473 on run 36109529376). Restore the test's intent -- verify that ``close_async`` releases an allocated context -- by explicitly calling ``await channel.transport.connect()`` inside the ``async with``. That forces the ``Context`` into existence so ``__aexit__``'s ``close_async`` has something to close, which is exactly the wedge scenario the test is guarding against. --- .../transport/zeromq/test_close_async_audit.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/pytests/functional/transport/zeromq/test_close_async_audit.py b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py index 16cadd12666f..6b77db90dd5c 100644 --- a/tests/pytests/functional/transport/zeromq/test_close_async_audit.py +++ b/tests/pytests/functional/transport/zeromq/test_close_async_audit.py @@ -137,14 +137,15 @@ async def _drive(): async with salt.channel.client.AsyncReqChannel.factory( opts, crypt="clear" ) as channel: - # __aenter__ calls transport.connect() which allocates the - # asyncio Context on demand. Grab a strong ref for the - # post-close assertion. + # ``__aenter__`` is lazy (matches the sync ``__enter__`` + # shape) so no ``zmq.asyncio.Context`` gets allocated + # until the first send/connect. Force ``connect()`` + # here so ``__aexit__``'s ``close_async`` has an + # allocated context to release -- that is the wedge-risk + # scenario this test guards. + await channel.transport.connect() transport = channel.transport contexts_seen.append(transport.context) - # No send -- the wedge is a teardown problem, and we want - # to verify teardown discipline on a channel that only ever - # connected. asyncio.run(_drive())