Defer anyio.Lock creation via cached_property to bind to running loop#5265
Defer anyio.Lock creation via cached_property to bind to running loop#5265
anyio.Lock creation via cached_property to bind to running loop#5265Conversation
…loop Replace eager `asyncio.Lock` instantiation at class/instance-init time with `functools.cached_property`-wrapped `anyio.Lock` across `Agent`, `MCPServer`, `FallbackModel`, `Provider`, `_VertexAIAuth`, and `FastMCPToolset`. `anyio.Lock` binds to the event loop on which it's first used, so deferring creation until first access ensures it binds to the correct running loop and avoids issues with Temporal's workflow sandbox (which blocks dynamic imports and certain initialization happening inside workflow code).
github-actions
Bot
added
size: M
|
|
||
| server._enter_lock = InterleavingLock() # pyright: ignore[reportPrivateUsage] | ||
| # `_enter_lock` is a `cached_property`; seeding the instance dict primes the cache. | ||
| vars(server)['_enter_lock'] = InterleavingLock() |
There was a problem hiding this comment.
This test suite has a lot of private object testing. This is not good.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95cdfebfbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._entered_count -= 1 | ||
| if self._entered_count == 0 and self._own_http_client is not None: |
There was a problem hiding this comment.
__aexit__ when provider was never entered
Provider.__aexit__() now always decrements _entered_count, even when __aenter__() was never called. This regresses the previously supported no-op behavior (see tests/test_agent.py::test_provider_aexit_without_aenter) and can corrupt lifecycle state: after an early __aexit__, _entered_count becomes -1, so the next real async with provider: path skips the == 0 enter logic and later exits will no longer close the owned HTTP client, causing a resource leak.
Useful? React with 👍 / 👎.
Docs Preview
|
The previous `_enter_lock is None` guard short-circuited `__aexit__` when `__aenter__` had never been called. Switching to a `cached_property` removed that guard and allowed `_entered_count` to drift to -1, which then caused a later `async with` to skip the close branch and leak the owned HTTP client. Add an explicit early return when `_entered_count == 0`, plus a regression test that re-enters the provider after a bare `__aexit__`.
- Subclassing `anyio.Lock` is silently bypassed because `anyio.Lock` returns a backend adapter via `__new__`, so the override's body was never actually executed. Replace the subclass with a duck-typed wrapper that delegates to an inner `anyio.Lock` so the yield point is real. - `_VertexAIAuth._refresh_lock` is only consumed by `_refresh_token`, which is itself `# pragma: no cover`. Mark the cached_property body the same way.
1 participant
Replaces eager
asyncio.Lockinstantiation at class/instance-init time withfunctools.cached_property-wrappedanyio.LockacrossAgent,MCPServer,FallbackModel,Provider,_VertexAIAuth, andFastMCPToolset.anyio.Lockbinds to the event loop on which it's first used, so deferring creation until first access ensures it binds to the correct running loop and avoids issues with Temporal's workflow sandbox (which blocks dynamic imports and initialization happening inside workflow code).Picks up the lock-related portion of #3012, leaving the broader asyncio→anyio migration for a follow-up that won't conflict with the agent graph refactor.
Checklist
AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.