Repository navigation
Forward POSIX shutdown signals to supervised processes in JSON mode - #7558
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
[Medium impact] The reviewed changes appear safe to merge, with no new blocking findings. SummaryThis PR forwards SIGINT to the supervised child on POSIX when the JSON supervisor catches a keyboard interrupt.
Reviews (9) · Last reviewed commit: "Merge branch 'main' into fix/7509-json-r..." · Reviewed by Greptile |
Merging this PR will improve performance by 4.01%2 warnings
Performance Changes
Tip Curious why performance improved? Comment Comparing |
There was a problem hiding this comment.
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
masenf
left a comment
There was a problem hiding this comment.
thanks. this is important. i just want to make sure we're not introducing a hard to debug future problem
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.
Comments Outside DiffThese findings could not be posted inline.
|
Summary
When
reflex run --jsonsupervises 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 withstopsignal=INTcan stop Reflex cleanly.Closes #7509.
Testing
uv run pytest tests/units/reflex_base/utils/test_log.py -q— 80 passed, 2 skipped on Windows.