fix(tools): reuse persistent loop in Home Assistant async bridge#13422
Open
stepanov1975 wants to merge 2 commits into
Open
fix(tools): reuse persistent loop in Home Assistant async bridge#13422stepanov1975 wants to merge 2 commits into
stepanov1975 wants to merge 2 commits into
Conversation
3fca7db to
df04c30
Compare
11 tasks
Contributor
Author
|
@0xbyt4 sorry for the direct ping — I’m trying not to create review noise, but this PR has been sitting without checks because it needs maintainer workflow approval. I believe the underlying issue is still present on current If this PR fix not good enough, may be you can look at the issue and create better fix as issue still need fix. |
Keep homeassistant_tool._run_async() on persistent event loops across main-thread, worker-thread, and already-running-loop call modes. Replace the remaining per-call asyncio.run() path with a background bridge loop and add regression coverage for running-loop reuse.
df04c30 to
2f9f232
Compare
2f9f232 to
eb104e9
Compare
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
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?
This PR fixes the remaining async-context gap in
tools/homeassistant_tool.py's sync-to-async bridge.The Home Assistant tool already reused persistent event loops for some sync call paths, but when
_run_async()was invoked from inside an already-running event loop it still spawned a disposable thread and calledasyncio.run(coro)per invocation. That per-call create-and-destroy loop behavior can leaveaiohttpcleanup tied to dead loops, which risksUnclosed client session/Unclosed connectorwarnings in long-running Hermes processes.This change makes that path reuse a dedicated persistent background loop thread instead, bringing the Home Assistant tool in line with Hermes' persistent-loop bridge pattern.
Related Issue
Fixes #13586
Type of Change
Changes Made
tools/homeassistant_tool.pyso async-context calls no longer use per-callasyncio.run()asyncio.run_coroutine_threadsafe(...)tests/tools/test_homeassistant_tool.pyfor loop reuse and liveness from inside a running event loopHow to Test
tools/homeassistant_tool.py:_run_async()from within an already-running event loop.asyncio.run()loop creation/destruction behavior.python -m pytest tests/tools/test_homeassistant_tool.py -q -o 'addopts='python -m pytest tests/gateway/test_homeassistant.py -q -o 'addopts='pytest tests/ -q.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Targeted local test results:
tests/tools/test_homeassistant_tool.py: passedtests/gateway/test_homeassistant.py: passedNote: I did not mark the full
pytest tests/ -qcheckbox as complete here because repository-wide baseline failures unrelated to this PR were observed separately.