Fix master-initiated jobs on Py3.12 by installing asyncio loop in SyncWrapper (#65702)#69482
Open
dwoz wants to merge 2 commits into
Open
Fix master-initiated jobs on Py3.12 by installing asyncio loop in SyncWrapper (#65702)#69482dwoz wants to merge 2 commits into
dwoz wants to merge 2 commits into
Conversation
…cWrapper On Python 3.12+ the SyncWrapper worker thread had no asyncio event loop installed, so any wrapped coroutine that touched ``asyncio.get_event_loop()`` raised ``RuntimeError: There is no current event loop in thread 'Thread-N (_target)'`` and aborted the call. This breaks every master-initiated job (``salt '*' test.ping``, ``state.apply``, etc.) on the salt master CLI because ``LocalClient.pub`` sends through ``AsyncReqChannel`` wrapped by ``SyncWrapper``, and the underlying pyzmq ``zmq.eventloop.future`` socket calls ``asyncio.get_event_loop()`` from the worker thread. Python 3.12 removed the implicit auto-creation of an event loop in non-main threads, so ``get_event_loop`` raises instead of silently constructing one. Create a dedicated ``asyncio.new_event_loop()`` per SyncWrapper and install it on the worker thread via ``asyncio.set_event_loop()`` before ``io_loop.run_sync(...)``. Close the asyncio loop in ``SyncWrapper.close``. Fixes saltstack#65702
The top-level 'import asyncio' added in the previous commit broke salt-ssh tests against the centos py36 target container: that thin tarball does not ship 'typing_extensions', so the asyncio -> contextvars -> immutables -> typing_extensions import chain fails at module load. Move the import inside SyncWrapper.__init__ and SyncWrapper._target where it is actually used, leaving the rest of salt.utils.asynchronous importable on legacy py3.6/3.7 ssh thin targets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes
salt-CLI publishes failing on Python 3.12+ withRuntimeError: There is no current event loop in thread 'Thread-N (_target)'.salt.utils.asynchronous.SyncWrapperspawns a worker thread(
threading.Thread(target=self._target, ...)-> Python names it"Thread-N (_target)") and runsio_loop.run_sync(...)inside it.Any wrapped coroutine that touches
asyncio.get_event_loop()-notably pyzmq's
zmq.eventloop.futuresockets used by everyLocalClient.pubsend throughAsyncReqChannel- hits that callfrom the worker thread, and on Python 3.12+ it raises because the
implicit per-thread loop creation was removed.
Create a dedicated
asyncio.new_event_loop()perSyncWrapperandinstall it on the worker thread via
asyncio.set_event_loop()before
io_loop.run_sync(...). Close the asyncio loop inSyncWrapper.close. The vendored Tornado IOLoop on 3006.x continuesto drive the coroutine itself; this change only ensures
asyncio.get_event_loop()returns a usable loop on the workerthread for any code path that asks for one.
What issues does this PR fix or reference?
Fixes #65702
Previous Behavior
On Python 3.12+ (Fedora 39+, Ubuntu 24.04, etc.), running
salt '*' test.pingorsalt '*' state.applyon the salt masterproduced:
and the publish was dropped, so no minion ever ran the job. Every
master-initiated job collapsed.
New Behavior
SyncWrapperinstalls an asyncio event loop on its worker thread,so pyzmq's
zmq.eventloop.futuresocket calls (and any other codethat calls
asyncio.get_event_loop()) succeed and the publishgoes through normally.
Regression test
tests/pytests/unit/utils/test_asynchronous.py:: test_sync_wrapper_thread_has_asyncio_loop_65702constructs aSyncWrapperaround a coroutine that callsasyncio.get_event_loop()from insideio_loop.run_sync. Withoutthis patch it fails on Py3.12+ with the exact reported error.
With the patch it passes on both Py3.10 and Py3.12.
Merge requirements satisfied?
changelog/65702.fixed.md)Commits signed with GPG?
Yes