Skip to content

Handle UnpicklingError during worker shutdown#3145

Open
pmontepagano wants to merge 2 commits into
sanic-org:mainfrom
pmontepagano:worker-shutdown-race-condition
Open

Handle UnpicklingError during worker shutdown#3145
pmontepagano wants to merge 2 commits into
sanic-org:mainfrom
pmontepagano:worker-shutdown-race-condition

Conversation

@pmontepagano

Copy link
Copy Markdown

When SIGTERM arrives during a _sync_states() call, Python's signal handler interrupts the blocking read() on the multiprocessing proxy connection. The shutdown handler then accesses the same connection (via set_state()), and _ForkingPickler.loads() sees corrupt bytes from the interrupted read, raising _pickle.UnpicklingError.

This adds UnpicklingError to the existing exception handling in:

  • WorkerProcess.terminate() — alongside BrokenPipeError, ConnectionResetError, EOFError
  • WorkerProcess.exit() — same tuple, for cleanup consistency
  • WorkerManager._sync_states() — alongside KeyError

These follow the same pattern already used for other connection-related exceptions during shutdown.

Related: https://community.sanicframework.org/t/random-exceptions-with-workermanager/1154

@pmontepagano pmontepagano marked this pull request as ready for review March 11, 2026 19:47
@pmontepagano pmontepagano requested a review from a team as a code owner March 11, 2026 19:47
@codecov

codecov Bot commented Mar 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.845%. Comparing base (785d77f) to head (9c23ff7).

Additional details and impacted files
@@              Coverage Diff              @@
##              main     #3145       +/-   ##
=============================================
+ Coverage   87.793%   87.845%   +0.052%     
=============================================
  Files          105       105               
  Lines         8143      8145        +2     
  Branches      1290      1290               
=============================================
+ Hits          7149      7155        +6     
+ Misses         687       685        -2     
+ Partials       307       305        -2     

☔ 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.

@themavik themavik left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Treating pickle.UnpicklingError like KeyError in _sync_states matches a broken multiprocessing proxy during shutdown. nit: pickle is only imported for the exception type—a short note beside nosec keeps bandit from looking for loads().

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.

2 participants