fix: replace deprecated asyncio.get_event_loop() with new_event_loop()#3266
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the server startup code to avoid deprecated asyncio.get_event_loop() usage by creating explicit event loops, and adds a regression test to prevent reintroduction of bare get_event_loop() calls.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()(+set_event_loop) inrun_tasks(). - Replace
--freshteardown loop usage with a dedicatedasyncio.new_event_loop()that is closed afterward. - Add a security test to enforce the absence of bare
asyncio.get_event_loop()calls inserver.py.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server.py |
Switches server startup and --fresh teardown to explicit event loop creation instead of get_event_loop(). |
tests/security/test_asyncio_event_loop.py |
Adds a regression test intended to prevent reintroducing deprecated event loop usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
76
to
+78
| def run_tasks(services, run_vue_server=False): | ||
| loop = asyncio.get_event_loop() | ||
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) |
Comment on lines
+317
to
+319
| loop.run_until_complete(data_svc.destroy()) | ||
| loop.run_until_complete(knowledge_svc.destroy()) | ||
| loop.close() |
Comment on lines
+2
to
+24
|
|
||
|
|
||
| class TestAsyncioEventLoop(unittest.TestCase): | ||
| def test_no_bare_get_event_loop_in_server(self): | ||
| """Verify no bare asyncio.get_event_loop() calls remain in server.py.""" | ||
| with open('server.py', 'r') as f: | ||
| lines = f.readlines() | ||
| for i, line in enumerate(lines, 1): | ||
| stripped = line.strip() | ||
| if 'asyncio.get_event_loop()' in stripped and not stripped.startswith('#'): | ||
| self.fail(f'Found bare asyncio.get_event_loop() at line {i}: {stripped}') | ||
|
|
||
| def test_new_event_loop_in_run_tasks(self): | ||
| """Verify run_tasks uses asyncio.new_event_loop().""" | ||
| with open('server.py', 'r') as f: | ||
| content = f.read() | ||
| start = content.find('def run_tasks') | ||
| end = content.find('\ndef ', start + 1) | ||
| func_content = content[start:end] | ||
| self.assertIn('asyncio.new_event_loop()', func_content) | ||
| self.assertIn('asyncio.set_event_loop(loop)', func_content) | ||
|
|
||
|
|
deacon-mp
force-pushed
the
fix/asyncio-run-replacement
branch
2 times, most recently
from
a546e7d to
eae2d27
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the server startup path to avoid deprecated asyncio.get_event_loop() usage by explicitly creating new event loops, and adds a security-oriented regression test to prevent reintroducing the deprecated pattern in server.py.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()+asyncio.set_event_loop()inrun_tasks(). - Add loop shutdown/close cleanup in
run_tasks()and update the--freshstartup path to use a dedicated loop. - Add a test that inspects
server.pyto enforce the new event loop pattern and prevent bareget_event_loopusage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
server.py |
Switches server startup and --fresh teardown to use explicitly created event loops and adds loop cleanup. |
tests/security/test_asyncio_event_loop.py |
Adds AST-based checks intended to prevent reintroduction of asyncio.get_event_loop() in server.py. |
Comments suppressed due to low confidence (1)
server.py:83
run_tasks()creates and uses the event loop for a long sequence ofrun_until_complete()calls before thetry/finally. If any of those earlier calls raise (e.g., restore/load failures), the loop will never be closed. Wrap the entire function body (starting right after loop creation) in atry/finallyso the loop is always shut down/closed on any exception, not only afterrun_forever()exits.
def run_tasks(services, run_vue_server=False):
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
loop.create_task(app_svc.validate_requirements())
loop.run_until_complete(data_svc.restore_state())
loop.run_until_complete(knowledge_svc.restore_state())
loop.run_until_complete(app_svc.register_contacts())
loop.run_until_complete(app_svc.load_plugins(args.plugins))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+127
to
+129
| finally: | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) | ||
| loop.close() |
| try: | ||
| loop.run_until_complete(data_svc.destroy()) | ||
| loop.run_until_complete(knowledge_svc.destroy()) | ||
| finally: |
Comment on lines
+34
to
+42
| content = SERVER_PY.read_text() | ||
| tree = _parse_server() | ||
| func_node = _get_function(tree, 'run_tasks') | ||
| assert func_node is not None, 'run_tasks not found in server.py' | ||
| func_src = ast.get_source_segment(content, func_node) | ||
| assert 'asyncio.new_event_loop()' in func_src | ||
| assert 'asyncio.set_event_loop(loop)' in func_src | ||
|
|
||
|
|
| tree = _parse_server() | ||
| func_node = _get_function(tree, 'run_tasks') | ||
| assert func_node is not None, 'run_tasks not found in server.py' | ||
| func_src = ast.get_source_segment(content, func_node) |
Comment on lines
+24
to
+29
| if isinstance(node, ast.Attribute) and node.attr == 'get_event_loop': | ||
| if isinstance(node.value, ast.Name) and node.value.id == 'asyncio': | ||
| lineno = node.lineno | ||
| line = content.splitlines()[lineno - 1].strip() | ||
| assert line.startswith('#'), \ | ||
| f'Found bare asyncio.get_event_loop() at line {lineno}: {line}' |
Use asyncio.new_event_loop() in run_tasks() and --fresh block.
…k; fix tests to use ast+pytest
- Replace brittle substring matching with AST Call node inspection
via a shared _is_asyncio_call() helper
- Remove incorrect startswith('#') guard (AST never includes comments)
- Eliminate ast.get_source_segment() to avoid potential None return
deacon-mp
force-pushed
the
fix/asyncio-run-replacement
branch
from
bc4e8da to
6d63ba8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates server startup/shutdown to avoid deprecated asyncio.get_event_loop() usage by explicitly creating fresh event loops, and adds a test to enforce that server.py no longer uses bare get_event_loop() calls.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()+asyncio.set_event_loop()inrun_tasks(). - Add loop shutdown/close logic to
run_tasks()and create a dedicated loop for the--freshreset block. - Add an AST-based test to ensure
server.pyno longer containsasyncio.get_event_loop()and thatrun_tasks()uses/cleans up a new loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
server.py |
Switches server startup to explicit new event loops; adds shutdown/close handling and a fresh-start loop. |
tests/security/test_asyncio_event_loop.py |
Adds AST inspection tests enforcing loop API usage rules in server.py. |
Comments suppressed due to low confidence (1)
server.py:103
- The loop cleanup (shutdown_asyncgens/close) is only in the try/finally that wraps run_forever. If any of the earlier run_until_complete(...) setup calls raises before reaching that try block, the newly created loop will never be closed. Wrap the entire run_tasks body (after loop creation) in a broader try/finally so the loop is always shutdown/closed on all exceptions during startup.
def run_tasks(services, run_vue_server=False):
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
loop.create_task(app_svc.validate_requirements())
loop.run_until_complete(data_svc.restore_state())
loop.run_until_complete(knowledge_svc.restore_state())
loop.run_until_complete(app_svc.register_contacts())
loop.run_until_complete(app_svc.load_plugins(args.plugins))
loop.run_until_complete(
data_svc.load_data(
loop.run_until_complete(data_svc.locate("plugins", dict(enabled=True)))
)
)
loop.run_until_complete(
app_svc.load_plugin_expansions(
loop.run_until_complete(data_svc.locate("plugins", dict(enabled=True)))
)
)
loop.run_until_complete(RestApi(services).enable())
loop.run_until_complete(auth_svc.set_login_handlers(services))
loop.create_task(app_svc.start_sniffer_untrusted_agents())
loop.create_task(app_svc.resume_operations())
loop.create_task(app_svc.run_scheduler())
loop.create_task(learning_svc.build_model())
loop.create_task(app_svc.watch_ability_files())
loop.run_until_complete(start_server())
loop.run_until_complete(event_svc.fire_event(exchange="system", queue="ready"))
loop.run_until_complete(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| finally: | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) | ||
| loop.close() |
Comment on lines
+319
to
+324
| loop = asyncio.new_event_loop() | ||
| try: | ||
| loop.run_until_complete(data_svc.destroy()) | ||
| loop.run_until_complete(knowledge_svc.destroy()) | ||
| finally: | ||
| loop.close() |
Comment on lines
+54
to
+66
| def test_run_tasks_closes_loop(): | ||
| """Verify run_tasks closes the event loop in a finally block.""" | ||
| _, tree = _parse_server() | ||
| func_node = _get_function(tree, 'run_tasks') | ||
| assert func_node is not None, 'run_tasks not found in server.py' | ||
| has_finally_close = False | ||
| for node in ast.walk(func_node): | ||
| if isinstance(node, ast.Try): | ||
| for handler in node.finalbody: | ||
| for child in ast.walk(handler): | ||
| if (isinstance(child, ast.Attribute) and child.attr == 'close' | ||
| and isinstance(child.value, ast.Name) and child.value.id == 'loop'): | ||
| has_finally_close = True |
Comment on lines
+77
to
+78
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the server startup/shutdown path to avoid deprecated asyncio.get_event_loop() usage by explicitly creating and managing event loops, and adds a regression test to enforce this in server.py.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()+asyncio.set_event_loop(...)inserver.pystartup (run_tasks) and--freshreset flow. - Add AST-based tests to ensure
server.pyno longer usesasyncio.get_event_loop()and thatrun_taskssets/closes its loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server.py |
Switches to explicit loop creation/teardown for startup and --fresh reset. |
tests/security/test_asyncio_event_loop.py |
Adds AST inspections to enforce the no-get_event_loop() rule in server.py and validate loop lifecycle calls in run_tasks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
76
to
+82
| def run_tasks(services, run_vue_server=False): | ||
| loop = asyncio.get_event_loop() | ||
| loop.create_task(app_svc.validate_requirements()) | ||
| loop.run_until_complete(data_svc.restore_state()) | ||
| loop.run_until_complete(knowledge_svc.restore_state()) | ||
| loop.run_until_complete(app_svc.register_contacts()) | ||
| loop.run_until_complete(app_svc.load_plugins(args.plugins)) | ||
| loop.run_until_complete( | ||
| data_svc.load_data( | ||
| loop.run_until_complete(data_svc.locate("plugins", dict(enabled=True))) | ||
| ) | ||
| ) | ||
| loop.run_until_complete( | ||
| app_svc.load_plugin_expansions( | ||
| loop.run_until_complete(data_svc.locate("plugins", dict(enabled=True))) | ||
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) | ||
| try: | ||
| loop.create_task(app_svc.validate_requirements()) | ||
| loop.run_until_complete(data_svc.restore_state()) | ||
| loop.run_until_complete(knowledge_svc.restore_state()) |
| loop.run_until_complete( | ||
| services.get("app_svc").teardown(main_config_file=args.environment) | ||
| ) | ||
| finally: |
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to remove deprecated asyncio.get_event_loop() usage from server.py by explicitly creating/setting a new event loop in run_tasks() and in the --fresh startup path, and adds a test that enforces the absence of get_event_loop() calls in server.py.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()+asyncio.set_event_loop()inserver.pystartup paths. - Add shutdown logic to cancel pending tasks and close the loop in
run_tasks(). - Add an AST-based test to ensure
server.pyno longer callsasyncio.get_event_loop()and thatrun_tasks()uses/cleans up a new loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server.py |
Switches loop creation/selection to new_event_loop() and adds cleanup/cancellation on shutdown (also updates --fresh loop handling). |
tests/security/test_asyncio_event_loop.py |
Adds AST-based tests to enforce no get_event_loop() usage in server.py and to check run_tasks() loop setup/cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+78
to
+84
| # The event loop is set here, before any async work begins. Services | ||
| # (AppService, DataService, etc.) are instantiated in __main__ prior to | ||
| # this call but they do not cache the loop at construction time — they | ||
| # resolve it lazily via asyncio.get_event_loop(). Setting it explicitly | ||
| # here ensures all subsequent loop.create_task() / loop.run_until_complete() | ||
| # calls operate on the same, controlled loop instance. | ||
| asyncio.set_event_loop(loop) |
Comment on lines
+135
to
+142
| # Cancel all pending tasks before shutdown to avoid resource leaks | ||
| # and "Task was destroyed but it is pending!" warnings. | ||
| pending = asyncio.all_tasks(loop) | ||
| for task in pending: | ||
| task.cancel() | ||
| if pending: | ||
| loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) |
Comment on lines
+42
to
+51
| """Verify run_tasks creates a new event loop and sets it via AST inspection.""" | ||
| _, tree = _parse_server() | ||
| func_node = _get_function(tree, 'run_tasks') | ||
| assert func_node is not None, 'run_tasks not found in server.py' | ||
|
|
||
| has_new_loop = any(_is_asyncio_call(n, 'new_event_loop') for n in ast.walk(func_node)) | ||
| has_set_loop = any(_is_asyncio_call(n, 'set_event_loop') for n in ast.walk(func_node)) | ||
|
|
||
| assert has_new_loop, 'run_tasks must call asyncio.new_event_loop()' | ||
| assert has_set_loop, 'run_tasks must call asyncio.set_event_loop(loop)' |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates server.py to avoid asyncio.get_event_loop() usage by explicitly creating and managing event loops, and adds a regression test to prevent reintroducing bare get_event_loop() calls in server.py.
Changes:
- Replace
asyncio.get_event_loop()usage inrun_tasks()and the--freshstartup block withasyncio.new_event_loop()+asyncio.set_event_loop(...). - Add event-loop shutdown/cleanup logic in
run_tasks()(cancel pending tasks, shutdown async generators, close loop). - Add AST-based tests to ensure
server.pydoes not useasyncio.get_event_loop()and thatrun_tasks()sets/closes a loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
server.py |
Switches to explicit loop creation/setting and adds loop cleanup on shutdown. |
tests/security/test_asyncio_event_loop.py |
Adds AST-based checks preventing bare get_event_loop() in server.py and asserting loop lifecycle handling in run_tasks(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+78
to
+84
| # The event loop is set here, before any async work begins. Services | ||
| # (AppService, DataService, etc.) are instantiated in __main__ prior to | ||
| # this call but they do not cache the loop at construction time — they | ||
| # resolve it lazily via asyncio.get_event_loop(). Setting it explicitly | ||
| # here ensures all subsequent loop.create_task() / loop.run_until_complete() | ||
| # calls operate on the same, controlled loop instance. | ||
| asyncio.set_event_loop(loop) |
Comment on lines
+78
to
+83
| # The event loop is set here, before any async work begins. Services | ||
| # (AppService, DataService, etc.) are instantiated in __main__ prior to | ||
| # this call but they do not cache the loop at construction time — they | ||
| # resolve it lazily via asyncio.get_event_loop(). Setting it explicitly | ||
| # here ensures all subsequent loop.create_task() / loop.run_until_complete() | ||
| # calls operate on the same, controlled loop instance. |
|
|
||
|
|
||
| def _parse_server(): | ||
| content = SERVER_PY.read_text() |
Comment on lines
+66
to
+67
| if (isinstance(child, ast.Attribute) and child.attr == 'close' | ||
| and isinstance(child.value, ast.Name) and child.value.id == 'loop'): |
|
❌ The last analysis has failed. |
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates server.py to avoid deprecated asyncio.get_event_loop() usage by explicitly creating and setting event loops, and adds AST-based tests to prevent regressions.
Changes:
- Replace
asyncio.get_event_loop()withasyncio.new_event_loop()+asyncio.set_event_loop()inrun_tasks()and the--freshstartup block. - Add explicit loop shutdown/cleanup logic (cancel pending tasks, shutdown async generators, close loop).
- Add security tests that statically assert
server.pyno longer callsasyncio.get_event_loop()and thatrun_tasks()sets/closes the loop.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
server.py |
Creates/sets explicit event loops for task execution and fresh startup; adds shutdown/cleanup logic. |
tests/security/test_asyncio_event_loop.py |
Adds AST-based tests to enforce no get_event_loop() calls remain and verify loop lifecycle handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+83
| # resolve it lazily via asyncio.get_event_loop(). Setting it explicitly | ||
| # here ensures all subsequent loop.create_task() / loop.run_until_complete() | ||
| # calls operate on the same, controlled loop instance. |
Comment on lines
+137
to
+141
| pending = asyncio.all_tasks(loop) | ||
| for task in pending: | ||
| task.cancel() | ||
| if pending: | ||
| loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) |
| task.cancel() | ||
| if pending: | ||
| loop.run_until_complete(asyncio.gather(*pending, return_exceptions=True)) | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) |
Comment on lines
+334
to
+342
| loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(loop) | ||
| try: | ||
| loop.run_until_complete(data_svc.destroy()) | ||
| loop.run_until_complete(knowledge_svc.destroy()) | ||
| finally: | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) | ||
| loop.close() | ||
| asyncio.set_event_loop(None) |
fionamccrae
pushed a commit
that referenced
this pull request
Mar 16, 2026
#3266) * fix: replace deprecated asyncio.get_event_loop() with new_event_loop() Use asyncio.new_event_loop() in run_tasks() and --fresh block. * fix: close event loop in finally block; use try/finally in fresh block; fix tests to use ast+pytest * fix: remove unused variable in asyncio test (flake8 F841) * test: rewrite asyncio event loop tests to use pure AST inspection - Replace brittle substring matching with AST Call node inspection via a shared _is_asyncio_call() helper - Remove incorrect startswith('#') guard (AST never includes comments) - Eliminate ast.get_source_segment() to avoid potential None return * fix: ensure event loop is always closed and cleared on all exit paths * fix: cancel pending tasks before closing event loop in run_tasks * test: relax event loop assertion to allow non-new_event_loop refactors * test: use explicit utf-8 encoding in read_text()
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.
Use asyncio.new_event_loop() in run_tasks() and --fresh block.
Description
(insert summary)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: