Cancel activities Core stopped tracking at worker shutdown - #1837
Draft
DABH wants to merge 3 commits into
Draft
Conversation
When a workflow run is evicted while one of its local activities is executing, Core queues a cancel for the activity and then, once the eviction activation completes, invalidates the run, which removes the activity from Core's outstanding set. If the cancel is still queued at that point Core discards it as no longer tracked, while the Python worker keeps running the activity. Core then reports activity polling as finished and wait_all_completed waits for that task forever, so worker shutdown hangs. Once Core's activity poll has shut down it tracks no activity, so anything still executing can only be finished from here. Cancel it with worker_shutdown cancellation details, log a warning, and let its completion be ignored by Core as untracked. This is the shutdown hang behind the frequent macOS CI timeouts of test_workflow_cancel_activity[True], whose captured logs show a workflow task eviction followed by two local activity starts for one run and one fewer cancel. The regression test forces the same race by delaying the activity poll that follows a local activity start and terminating the workflow so the workflow task heartbeat fails.
There was a problem hiding this comment.
🟡 Changes recommended
Existing cancellation details can be overwritten, and the regression assertion does not guarantee the new fallback ran.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Prevents worker shutdown from hanging on activities no longer tracked by Core.
Changes:
- Cancels orphaned activities after polling stops.
- Adds a regression test and changelog entry.
File summaries
| File | Description |
|---|---|
temporalio/worker/_activity.py |
Cancels remaining activities during shutdown. |
tests/worker/test_workflow.py |
Tests orphaned local-activity shutdown. |
CHANGELOG.md |
Documents the fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+205
to
+209
| activity.cancellation_details.details = ( | ||
| temporalio.activity.ActivityCancellationDetails( | ||
| worker_shutdown=True | ||
| ) | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Agreed. In 2c5b821 the shutdown path only sets worker_shutdown details when the activity has none yet, and a new test (test_worker_shutdown_keeps_details_of_local_activity_ignoring_cancel) covers an activity that already received cancel_requested and keeps it.
| await asyncio.wait_for(worker.shutdown(), 20) | ||
| await run_task | ||
| assert len(details) == 1 and details[0] | ||
| assert details[0].worker_shutdown or details[0].cancel_requested |
Contributor
Author
There was a problem hiding this comment.
Done in 2c5b821. The test now forces the untracked ordering with events instead of sleeps (the activity poll is held until Core's workflow poll reports shutdown, so the queued cancel provably cannot be delivered) and asserts exactly [ActivityCancellationDetails(worker_shutdown=True)]. The other ordering has its own test with an exact assertion.
Comment on lines
+55
to
+57
| - Worker shutdown no longer waits forever for an activity that Core has stopped tracking, such as a | ||
| local activity whose cancellation was lost when its workflow run was evicted. Once activity polling | ||
| has shut down, any activity still executing is cancelled with `worker_shutdown` cancellation details. |
Contributor
Author
There was a problem hiding this comment.
Reworded in 2c5b821: the entry now says the activity is cancelled with worker_shutdown details only if it has none yet.
The regression test forced the lost-cancel race with wall-clock timing: a 3s delay on the activity poll that follows a local activity start and a 4s wait after terminating the workflow. On the slow fips CI job shutdown still hung for its 20s limit twice, and the fallback cancel never ran, so the worker was still waiting for its pollers to exit when the test gave up. Gate the test on events instead. The activity poll after the local activity start is held until the workflow poller has returned shutdown: Core only ends the workflow stream once it has processed the eviction completion that invalidates the run, and it marks workflows as shut down for the local activity manager before returning, so the cancel queued at eviction can never reach the worker and shutdown has to cancel the activity itself. The assertion now requires worker_shutdown details. A second test covers the other ordering: the cancel is delivered while Core still tracks the activity, the activity ignores it, and eviction is held until then so the run is invalidated afterwards. Shutdown must still end the activity without replacing its cancel_requested details, which are set once, so the fallback now only sets worker_shutdown details when none were recorded and the changelog entry says so. The workflow task timeout is 3s so that on a loaded runner the task cannot time out before the terminate lands, which replays the workflow and starts a second local activity.
DABH
force-pushed
the
flake/orphaned-activity-shutdown
branch
from
September 10, 2026 08:15
b2b1bc7 to
2c5b821
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 was changed
After activity polling has shut down, the worker cancels any activity still running, with a warning and, if it has no cancellation details yet,
worker_shutdowncancellation details, before waiting for completions. Regression tests and CHANGELOG entry.Why
On a workflow eviction Core queues a cancel for a running local activity, then invalidates the run and drops the activity from its outstanding set, so the queued cancel is discarded as untracked while Python keeps running the activity. Core then reports shutdown complete and
wait_all_completedwaits forever; this is the 60s timeout intest_workflow_cancel_activity[True]across 17 CI attempts. Once polling is down Core tracks nothing, so anything still running is orphaned. The lost cancel itself is a Core race worth an sdk-core issue.Testing
Two tests force each ordering with events instead of sleeps. One holds the activity poll until the workflow poller has shut down, so the queued cancel is never delivered and shutdown must cancel the activity with
worker_shutdowndetails. The other delivers the cancel first, has the activity ignore it, and checks that the shutdown cancel keeps itscancel_requesteddetails. Both hang for the 20s limit without the fix; 100/100 and 60/60 runs under heavy load on Python 3.14 and 3.10. Lint clean.