Repository navigation
fix: Exit 0 on SIGTERM while serving - #430
Conversation
erezrokah
left a comment
erezrokah
left a comment
There was a problem hiding this comment.
The idle-server shutdown and lock-restoration tests pass, but the new signal handling can leave the process alive indefinitely when an RPC handler is blocked. Please add bounded shutdown behavior for this case before merging.
| def _stop_in_background(self, signum, frame): | ||
| threading.Thread(target=self.stop, daemon=True).start() |
There was a problem hiding this comment.
[P2] Bound process shutdown when an RPC handler is blocked
Routing SIGTERM exclusively through stop() can leave the plugin process alive indefinitely. server.stop(5) cancels the RPC but does not interrupt the Python handler; after run() returns, interpreter shutdown waits for the executor worker. With Init blocked on threading.Event().wait(), the base process terminated immediately on SIGTERM, while this head remained alive beyond the five-second grace period and ignored further SIGTERM/SIGINT signals because they only stopped the already-stopped server. A plugin blocked on upstream I/O can therefore require SIGKILL instead of exiting cleanly. Add a bounded shutdown path for active handlers and a subprocess regression test covering this case.
erezrokah
left a comment
erezrokah
left a comment
There was a problem hiding this comment.
The blocked-RPC case now exits successfully within the intended bound. However, synchronous stream flushing can block or raise before the forced exit occurs. Please make forced termination independent of stdout/stderr flushing.
| sys.stdout.flush() | ||
| sys.stderr.flush() |
There was a problem hiding this comment.
[P2] Keep stream flushing from preventing forced exit
Both forced-exit paths call these synchronous flushes before reaching os._exit(0). If a worker is blocked writing to a full stdout pipe, flush() waits on that buffered stream, so both the timeout thread and the second-signal handler hang. I reproduced the process remaining alive after 12 seconds and a second signal, with both stacks stopped at line 435. Closing the pipe instead caused BrokenPipeError in the shutdown thread, and a second signal produced exit code 120. Make flushing best-effort and bounded so neither blocking I/O nor a flush exception can prevent os._exit(0), and cover full/closed output pipes in the subprocess tests.
🤖 I have created a release *beep* *boop* --- ## [0.1.59](v0.1.58...v0.1.59) (2026-11-03) ### Bug Fixes * Exit 0 on SIGTERM while serving ([#430](#430)) ([b733e64](b733e64)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
|
Looks good, thanks!! |
Stop the gRPC server from a background thread on SIGTERM and SIGINT, and force exit 0 if a blocked RPC handler keeps the process alive past the grace period or a second signal arrives, so a plugin always exits 0 on shutdown (fixes cloudquery/cloudquery#23716).