Fix: tolerate non-main-thread event loops in signal-handler setup - #92
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prevents WorkflowEngine.create() from crashing when instantiated on an asyncio event loop running in a non-main thread, by tolerating add_signal_handler() failures in that environment and adding a regression test to cover the scenario.
Changes:
- Broaden
_setup_signal_handlers()exception handling to skip signal registration whenadd_signal_handler()fails off the main thread, and emit a warning when handlers are skipped. - Add a unit test that creates and shuts down an engine from an event loop running in a secondary thread.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/test_termination.py | Adds regression coverage for creating/shutting down an engine on a non-main-thread event loop. |
| src/radical/asyncflow/workflow_manager.py | Updates signal-handler setup to tolerate non-main-thread loops and warn when signal handlers are not installed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
WorkflowEngine.create() crashed when the event loop was running in a
secondary thread: _setup_signal_handlers() calls loop.add_signal_handler()
which raises RuntimeError ("set_wakeup_fd only works in main thread of the
main interpreter") off the main thread, while only NotImplementedError was
caught.
Skip signal handler installation upfront when the engine is not created on
the main thread, and warn once that the host process has to manage shutdown
itself. Exception handling inside the registration loop is left unchanged,
so genuine errors still surface. There is no matching
remove_signal_handler() site, so no teardown change is needed.
Adds a unit test creating and shutting down an engine on a loop in a
secondary thread.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
andre-merzky
force-pushed
the
fix/nonmain-thread-loop
branch
from
August 15, 2026 14:36
e1dbc17 to
d9f7ca0
Compare
AymenFJA
self-requested a review
August 19, 2026 00:46
AymenFJA
approved these changes
Aug 19, 2026
AymenFJA
left a comment
AymenFJA
left a comment
Collaborator
There was a problem hiding this comment.
Looks good to me thanks @andre-merzky
Collaborator
|
@andre-merzky feel free to merge assuming this is ready :) |
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.
Problem
WorkflowEngine.create()crashes with aRuntimeErrorwhen the engine is created on an event loop that does not run on the main thread:loop.add_signal_handler()raises (signal handlers can only be installed from the main thread), and_setup_signal_handlers()only tolerated the narrower exception raised on some platforms. This blocks embedding the engine in host applications that run plugins on worker-thread event loops (e.g. radical.orbit plugin hosts).Fix
_setup_signal_handlers()so the non-main-threadRuntimeErroris tolerated as well.Testing
mainwith the exactRuntimeErrorthis change addresses.tox -e py313: 92 passed / 2 skipped (baseline onmain: 91 passed / 2 skipped).ruff checkclean; theruff formatcomplaint onworkflow_manager.py(~line 794) is pre-existing onmainand untouched by this change.🤖 Generated with Claude Code