Skip to content

Check state in shutdown for handling uvloop double kill#3122

Merged
ahopkins merged 2 commits into
mainfrom
issue2948
Dec 31, 2025
Merged

Check state in shutdown for handling uvloop double kill#3122
ahopkins merged 2 commits into
mainfrom
issue2948

Conversation

@ahopkins

Copy link
Copy Markdown
Member

Closes #2948

Copilot AI review requested due to automatic review settings December 31, 2025 13:00
@ahopkins ahopkins requested a review from a team as a code owner December 31, 2025 13:00
@codecov

codecov Bot commented Dec 31, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.811%. Comparing base (a64dc64) to head (34b9ac3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sanic/server/runners.py 52.173% 11 Missing ⚠️
sanic/worker/process.py 40.000% 3 Missing ⚠️
sanic/mixins/startup.py 50.000% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##              main     #3122       +/-   ##
=============================================
- Coverage   87.957%   87.811%   -0.146%     
=============================================
  Files          105       105               
  Lines         8113      8139       +26     
  Branches      1287      1290        +3     
=============================================
+ Hits          7136      7147       +11     
- Misses         671       686       +15     
  Partials       306       306               

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses issue #2948 by adding robust error handling for connection failures during worker shutdown, particularly when processes are terminated rapidly (double kill scenario with uvloop).

Key changes:

  • Added exception handling for multiprocess connection errors (BrokenPipeError, ConnectionResetError, EOFError) across worker state operations
  • Introduced _run_shutdown_coro() function to handle event loop state issues during shutdown, with special handling for uvloop's behavior where the first run_until_complete() after loop.stop() may fail
  • Added signal handler cleanup before shutdown to prevent issues during the shutdown sequence

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
sanic/worker/process.py Extended exception handling in exit() to catch additional connection errors (BrokenPipeError, ConnectionResetError, EOFError); wrapped set_state() call in terminate() with similar exception handling to prevent crashes when monitor process has exited
sanic/server/runners.py Added _run_shutdown_coro() helper function to handle running shutdown coroutines when loop has been stopped; added signal handler removal before shutdown; wrapped app.set_serving(False) with exception handling for connection errors
sanic/mixins/startup.py Wrapped _get_process_states() method with exception handling to gracefully return empty list when shared state connection fails

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sanic/server/runners.py
Comment thread sanic/worker/process.py
Comment thread sanic/server/runners.py
Comment thread sanic/mixins/startup.py
Comment thread sanic/worker/process.py
Comment thread sanic/server/runners.py
Comment thread sanic/server/runners.py
@ahopkins ahopkins merged commit dc0939e into main Dec 31, 2025
48 of 51 checks passed
@ahopkins ahopkins deleted the issue2948 branch December 31, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RuntimeError: Event loop stopped before Future completed.

2 participants