Skip to content

Fix reap_process_group sudo fallback passing the process group id as the signal number - #71671

Open
ckarnell wants to merge 1 commit into
apache:mainfrom
ckarnell:fix/sudo-kill-missing-pid
Open

Fix reap_process_group sudo fallback passing the process group id as the signal number#71671
ckarnell wants to merge 1 commit into
apache:mainfrom
ckarnell:fix/sudo-kill-missing-pid

Conversation

@ckarnell

Copy link
Copy Markdown

reap_process_group has two sudo fallbacks for EPERM. The first passes a signal and the pids:

["sudo", "-n", "kill", "-" + str(int(sig))] + [str(p.pid) for p in all_processes_in_the_group]

The second, fourteen lines below, passes the process group id where the signal belongs and names no target at all:

subprocess.check_call(["sudo", "-n", "kill", "-" + str(process_group_id)])

That builds sudo -n kill -4242, which /bin/kill rejects with a usage error and exit 2, so check_call raises CalledProcessError and the process is never signalled. The path needs killpg to fail with ESRCH and then os.kill with EPERM, which is the run_as_user case where the group has already gone but the process has not.

Mocking both errnos and capturing the call:

before: ['sudo', '-n', 'kill', '-4242']
after:  ['sudo', '-n', 'kill', '-15', '4242']

#9202 reported this in 2020 against 1.10.9 and named reap_process_group. It was closed without a fix. The line is unchanged.

The added test fails on current main with ['sudo', '-n', 'kill', '-4242'] != ['sudo', '-n', 'kill', '-15', '4242'] and passes with the change. test_process_utils.py goes from 27 passing to 28. It mocks the signal calls, so it needs no database and carries no db_test marker.

@boring-cyborg

boring-cyborg Bot commented Aug 16, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

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.

1 participant