Skip to content

Closing asyncio task properly#6296

Open
marioaag wants to merge 1 commit into
avocado-framework:masterfrom
marioaag:closing_asyncio_task_properly
Open

Closing asyncio task properly#6296
marioaag wants to merge 1 commit into
avocado-framework:masterfrom
marioaag:closing_asyncio_task_properly

Conversation

@marioaag
Copy link
Copy Markdown

Prevent asyncio error messages for pending tasks destroyed at the end of the execution especially using Avocado's Job API.

Error message example:

Task was destroyed but it is pending!
task: <Task pending name='Task-1' coro=<StatusServer.serve_forever() running at /Users/mario/Library/Python/3.11/lib/python/site-packages/avocado/core/status/server.py:44> wait_for=<Future cancelled>>
Task was destroyed but it is pending!
task: <Task pending name='Task-2' coro=<Runner._update_status() running at /Users/mario/Library/Python/3.11/lib/python/site-packages/avocado/plugins/runner_nrunner.py:255> wait_for=<Future pending cb=[Task.task_wakeup()]>>

More information can be found in the conversation #6282

@mr-avocado mr-avocado Bot moved this to Review Requested in Default project Apr 10, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements proper cleanup for background asyncio tasks by storing task references and explicitly cancelling them during the shutdown of the run_suite method. The review feedback suggests using loop.create_task() instead of asyncio.ensure_future() to ensure consistent event loop usage and avoid potential scope issues. Additionally, it is recommended to refactor the duplicated cancellation logic into a loop for better maintainability and error handling.

Comment thread avocado/plugins/runner_nrunner.py Outdated
Comment thread avocado/plugins/runner_nrunner.py Outdated
Comment thread avocado/plugins/runner_nrunner.py Outdated
@marioaag marioaag force-pushed the closing_asyncio_task_properly branch 2 times, most recently from c25185e to 0b71a0f Compare April 10, 2026 23:00
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.32%. Comparing base (9aaeff1) to head (0b71a0f).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6296      +/-   ##
==========================================
- Coverage   72.32%   72.32%   -0.01%     
==========================================
  Files         206      206              
  Lines       23250    23259       +9     
==========================================
+ Hits        16816    16822       +6     
- Misses       6434     6437       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marioaag
Copy link
Copy Markdown
Author

@richtja @clebergnu could you help me with your review and if possible any hint why doc build is failing?

@harvey0100 harvey0100 self-requested a review April 24, 2026 13:21
Copy link
Copy Markdown
Contributor

@harvey0100 harvey0100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me -- the cleanup approach makes sense. Storing the task references and cancelling them in order (updater first, then server) before closing is the right way to handle this. I tested the pattern locally, and it cleanly resolves the pending task warnings.

On the loop.create_task() vs ensure_future thing Gemini raised -- I don't think it's a real issue here since loop is always defined by the time the cleanup code runs, but it'd be a nice-to-have for readability if you feel like changing it. Not blocking either way.

For the RTD failure -- don't worry about that, it's just a docs thing that was already fixed on master. A rebase should get you green CI.

Nice fix, thanks for tracking this down from #6282

@marioaag marioaag force-pushed the closing_asyncio_task_properly branch from 0b71a0f to b9eb5cc Compare April 24, 2026 22:11
@marioaag
Copy link
Copy Markdown
Author

@harvey0100, @richtja, any thing else that I can do to help to get this PR moving?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review Requested

Development

Successfully merging this pull request may close these issues.

2 participants