kaxil commented on PR #70950: URL: https://github.com/apache/airflow/pull/70950#issuecomment-5165642310
Thanks for the PR. A bare `setsid()` here would regress graceful shutdown: the official image runs dumb-init with `DUMB_INIT_SETSID=1`, which broadcasts SIGTERM to the whole process group on container stop, and that group co-membership is how the task subprocess receives the signal (see "Signal propagation" in the docker-stack entrypoint docs). With the child in its own session, the task never gets SIGTERM, so its `on_kill()` handler never runs before the grace period expires and everything is SIGKILLed. #69034 recently made "signal only the supervisor" an explicit opt-in via `DUMB_INIT_SETSID=0`; this change would force it everywhere. On the double-firing concern from #70949: `_reset_signals()` a few lines down already resets the inherited handlers to SIG_DFL in the child, so handlers can't fire twice. There's also an open PR covering this ground: #65738 moved from `os.setsid()` to `os.setpgid(0, 0)` during review, scoped it to task execution only, paired it with `os.killpg` in `kill()`, and removes this TODO. Closing in favor of that one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
