Skip to content

fix: Handle signal registration outside the main thread#218

Merged
igalshilman merged 4 commits into
restatedev:mainfrom
introspection-org:fix/uvloop-signal-handler
Jul 23, 2026
Merged

fix: Handle signal registration outside the main thread#218
igalshilman merged 4 commits into
restatedev:mainfrom
introspection-org:fix/uvloop-signal-handler

Conversation

@brightsparc

@brightsparc brightsparc commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • tolerate ValueError when an event loop rejects add_signal_handler()
  • add an ASGI health-request regression test that reproduces the rejection without depending on uvloop internals

Root cause

Restate installs a SIGTERM handler while serving the ASGI application. The existing code handles NotImplementedError and RuntimeError, which covers unsupported platforms and asyncio's behavior outside the main thread.

uvloop raises ValueError instead when add_signal_handler() is called outside the main thread. Granian runs each ASGI worker on its own thread, so Restate requests failed with:

ValueError: add_signal_handler() can only be called from the main thread

This was observed with Granian, uvloop, and free-threaded CPython 3.14.6. Switching Granian to the asyncio loop avoided the exception, but lost the desired uvloop runtime.

Impact

The additional exception handling lets Restate applications run under Granian worker threads with uvloop, including free-threaded Python deployments. Signal registration remains best-effort, matching the existing behavior for asyncio and unsupported platforms.

No Granian-specific or uvloop-specific runtime patch is introduced.

Validation

  • branch rebased on current restatedev/sdk-python:main
  • git diff --check passed
  • regression test added for a successful GET /restate/health when signal registration raises ValueError
  • local pytest execution was attempted, but the editable Rust extension build stalled in cargo metadata at zero CPU due to unrelated long-running Cargo jobs on the development machine; upstream CI should execute the focused test

@brightsparc brightsparc changed the title Handle signal registration outside the main thread fix: Handle signal registration outside the main thread Jul 23, 2026
@brightsparc
brightsparc marked this pull request as ready for review July 23, 2026 02:43
@brightsparc
brightsparc marked this pull request as draft July 23, 2026 02:43
@brightsparc
brightsparc marked this pull request as ready for review July 23, 2026 03:08

@igalshilman igalshilman 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.

Looks good!, thank you @brightsparc !

@igalshilman
igalshilman merged commit 5dfca1a into restatedev:main Jul 23, 2026
7 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 23, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants