Close worker background event loops - #256
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb27bb3-082b-4553-9941-7672749d7424
There was a problem hiding this comment.
Pull request overview
This PR fixes a resource-leak issue in the core SDK worker implementation by ensuring the background-thread asyncio event loop is properly closed, preventing delayed ResourceWarning: unclosed event loop emissions after worker shutdown.
Changes:
- Switch the worker thread loop ownership to
asyncio.run()so the event loop is created and deterministically closed when the worker thread exits. - Add a regression test that captures the worker thread’s event loop and asserts it is closed after the thread completes.
- Document the fix in the core
CHANGELOG.mdunderFIXED.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| durabletask/worker.py | Uses asyncio.run() in the worker background thread so the event loop is closed on thread exit. |
| tests/durabletask/test_worker_resiliency.py | Adds a regression test to verify the background event loop is closed after the worker thread exits. |
| CHANGELOG.md | Adds a user-facing changelog entry describing the event loop resource leak fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| worker.start() | ||
| worker._runLoop.join(timeout=1.0) | ||
|
|
||
| assert len(event_loops) == 1 | ||
| assert event_loops[0].is_closed() is True |
Bernd Verst (berndverst)
left a comment
Bernd Verst (berndverst)
left a comment
There was a problem hiding this comment.
Reviewed the event-loop lifecycle change, surrounding worker restart/stop behavior, regression coverage, compatibility, performance, changelog, and validation results. The change is correct, non-breaking, and appropriately closes worker-owned event-loop resources.
Why this is needed
TaskHubGrpcWorker.start()creates an asyncio event loop on its background thread but never closes it. Stopped workers therefore retain loop resources until cyclic garbage collection, which can emit a delayedResourceWarning: unclosed event loopduring unrelated code.This surfaced as the intermittent Python 3.11 failure in PR #252 when garbage collection ran inside a warning-capture block.
Changes
asyncio.run()to own and close the worker thread's event loopValidation
python -m pytest tests\durabletask\test_worker_resiliency.py tests\durabletask\test_worker_concurrency_loop.py tests\durabletask\test_worker_concurrency_loop_async.py --quietpython -m nox -s core_tests-3.11 -- tests\durabletask\test_worker_resiliency.py -k worker_start --quietpython -m flake8 durabletaskpython -m flake8 tests\durabletask