Skip to content

fix(zipapp): forward termination signals - #4161

Open
jpneufeld wants to merge 3 commits into
bazel-contrib:mainfrom
jpneufeld:fix/zipapp-signal-forwarding
Open

jpneufeld wants to merge 3 commits into
bazel-contrib:mainfrom
jpneufeld:fix/zipapp-signal-forwarding

Conversation

@jpneufeld

@jpneufeld jpneufeld commented Sep 15, 2026

Copy link
Copy Markdown

Executable ZIP launchers can keep an intermediate process alive for cleanup, so signals sent to the launcher do not reach the application. The same issue occurs when a zipapp is run as python app.pyz.

This change:

  • uses exec when the stage 2 bootstrap owns ZIP cleanup;
  • forwards SIGHUP, SIGINT, SIGQUIT, and SIGTERM from POSIX Python launchers;
  • keeps Popen.wait() as the only child reaper and preserves nonzero or signal-derived exit statuses; and
  • leaves Windows subprocess behavior unchanged.

Regression tests cover executable zipapps, python app.pyz, and both legacy ZIP bootstrap implementations. They verify handled SIGTERM, default SIGTERM, exit status 17, and existing ZIP behavior.

Fixes #3809

Testing:

  • bazel test --config=fast-tests //tests/py_zipapp:system_python_zipapp_test //tests/bootstrap_impls:run_binary_zip_yes_test //tests/bootstrap_impls:run_binary_bootstrap_script_zip_yes_test --runs_per_test=10 --test_output=errors
  • changed-file pre-commit hooks
  • shell syntax checks for both modified shell templates

— Generated by GPT-5.6-Sol

Use exec when stage 2 owns ZIP cleanup. For Python launchers, forward POSIX termination signals while keeping wait() as the sole child reaper so the application exit status is preserved.\n\nFixes bazel-contrib#3809
@aignas

aignas commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the PR, please:

  • Fix buildifier errors
  • Windows does not have signal.SIGKILL, so making the tests UNIX only could be a way forward.

Comment thread news/3809.fixed.md
Keep the existing cross-platform zipapp smoke test separate from signal tests that use SIGKILL. Also link the release note to issue bazel-contrib#3809.
@jpneufeld

Copy link
Copy Markdown
Author

Addressed in f816e365:

  • Moved the signal helpers and cases into a class guarded by @unittest.skipUnless(os.name == "posix", ...). The existing cross-platform smoke test remains separate.
  • Applied the issue-link suggestion to the news entry.
  • Ran both Buildifier hooks across the repository; buildifier and buildifier-lint pass locally. The previous CI job failed before scanning the repository because its helper tried to download nonexistent Buildifier v10.0.0, despite the v8.2.1 configuration. This push starts a new CI run.

— James' Codex

Use SIGTERM when cleaning up a leftover signal fixture. Windows Pyrefly analyzes the POSIX-only test class and does not define signal.SIGKILL.
@jpneufeld

Copy link
Copy Markdown
Author

Follow-up 780723a7 fixes the Windows Pyrefly failure. The class-level skip prevents runtime execution, but Pyrefly still analyzes signal.SIGKILL against the Windows stubs. Cleanup now sends SIGTERM; both fixtures terminate after that signal.

The Pyrefly-enabled Bazel build, Ruff, and all three focused signal targets pass locally. Fresh CI is running.

— James' Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zipapp doesn't forward signals (SIGTERM and friends) to the process, resulting in orphaned processes and no cleanup

2 participants