Skip to content

Forward POSIX shutdown signals to supervised processes in JSON mode - #7558

Merged
FarhanAliRaza merged 13 commits into
reflex-dev:mainfrom
harsh21234i:fix/7509-json-run-sigint
Oct 22, 2026
Merged

FarhanAliRaza merged 13 commits into
reflex-dev:mainfrom
harsh21234i:fix/7509-json-run-sigint

Conversation

@harsh21234i

@harsh21234i harsh21234i commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When reflex run --json supervises a child process, a SIGINT sent to the supervisor PID can interrupt the supervisor without stopping the child. Forward SIGINT to the running child on POSIX so process supervisors configured with stopsignal=INT can stop Reflex cleanly.

Closes #7509.

Testing

  • uv run pytest tests/units/reflex_base/utils/test_log.py -q — 80 passed, 2 skipped on Windows.
  • Linux PID-targeted SIGINT regression test — 1 passed.
  • Applicable pre-commit hooks — passed.

View guided diff

@harsh21234i
harsh21234i requested a review from a team as a code owner October 14, 2026 10:01

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/utils/log.py
Comment thread news/+7509-forward-sigint-to-supervised-process.bugfix.md Outdated
Comment thread news/+7509-forward-sigint-to-supervised-process.bugfix.md Outdated
@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge, with no new blocking findings.

Summary

This PR forwards SIGINT to the supervised child on POSIX when the JSON supervisor catches a keyboard interrupt.

  • JSON-mode runs pass a supervisor's SIGINT to the app child on POSIX.

Reviews (9) · Last reviewed commit: "Merge branch 'main' into fix/7509-json-r..." · Reviewed by Greptile

Comment thread news/+7509-forward-sigint-to-supervised-process.bugfix.md Outdated
Comment thread tests/units/reflex_base/utils/test_log.py Outdated
@codspeed

codspeed Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 4.01%

2 warnings

⚠️ 17 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 147 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_proxy_list_mutation 9.9 ms 9.5 ms +4.01%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing harsh21234i:fix/7509-json-run-sigint (febfe77) with main (74bcd8b)

Open in CodSpeed

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 4 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
@harsh21234i harsh21234i changed the title Fix/7509 json run sigint Forward POSIX shutdown signals to supervised processes in JSON mode Oct 14, 2026
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
@masenf masenf added the bug Something isn't working label Oct 19, 2026 — with ChatGPT Codex Connector
@masenf masenf added the on deck PRs lined up to review / merge next label Oct 22, 2026

@masenf masenf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks. this is important. i just want to make sure we're not introducing a hard to debug future problem

Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread packages/reflex-base/src/reflex_base/utils/log.py Outdated
Comment thread tests/units/reflex_base/utils/test_log.py Outdated
Comment thread tests/units/reflex_base/utils/test_log.py Outdated
Comment thread packages/reflex-base/news/7558.bugfix.md
Keep the child in the supervisor's process group so a group SIGKILL
still stops the whole app, and forward SIGINT from the KeyboardInterrupt
branch instead of replacing signal handlers. The app shuts down cleanly
when a terminal Ctrl-C reaches it twice.

Drop SIGHUP/SIGQUIT forwarding: granian reloads its workers on SIGHUP,
so forwarding it does not stop the app.
@greptile-apps

greptile-apps Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff

These findings could not be posted inline.

  • P1 Startup interrupts leave apps running packages/reflex-base/src/reflex_base/utils/log.py:518 ▶

    supervise_output now catches SIGINT only while waiting for the child. If a supervisor sends a PID-only interrupt after Popen starts the child but before the wait loop begins, KeyboardInterrupt escapes without signaling or stopping the child. The supervisor exits while the app keeps running. Keep interrupt forwarding active from child creation through the wait loop.

@FarhanAliRaza
FarhanAliRaza merged commit 05c087b into reflex-dev:main Oct 22, 2026
457 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working on deck PRs lined up to review / merge next

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reflex run --json ignores SIGINT sent to its pid; under supervisord stopsignal=INT the app is orphaned and blocks the restart

3 participants