Skip to content

Commit

Permalink
Avoid making it look like kill_process works on Windows
Browse files Browse the repository at this point in the history
This changes the code in Git.execute's local kill_process function,
which it uses as the timed callback for kill_after_timeout, to
remove code that is unnecessary because kill_process doesn't
support Windows, and to avoid giving the false impression that its
code could be used unmodified on Windows without serious problems.

- Raise AssertionError explicitly if it is called on Windows. This
  is done with "raise" rather than "assert" so its behavior doesn't
  vary depending on "-O".

- Don't pass process creation flags, because they were 0 except on
  Windows.

- Don't fall back to SIGTERM if Python's signal module doesn't know
  about SIGKILL. This was specifically for Windows which has no
  SIGKILL.

See #1756 for discussion.
  • Loading branch information
EliahKagan committed Dec 10, 2023
1 parent f42a63b commit 2f017ac
Showing 1 changed file with 7 additions and 11 deletions.
18 changes: 7 additions & 11 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -1015,11 +1015,9 @@ def execute(

def kill_process(pid: int) -> None:
"""Callback to kill a process."""
p = Popen(
["ps", "--ppid", str(pid)],
stdout=PIPE,
creationflags=PROC_CREATIONFLAGS,
)
if os.name == "nt":
raise AssertionError("Bug: This callback would be ineffective and unsafe on Windows, stopping.")
p = Popen(["ps", "--ppid", str(pid)], stdout=PIPE)
child_pids = []
if p.stdout is not None:
for line in p.stdout:
Expand All @@ -1028,18 +1026,16 @@ def kill_process(pid: int) -> None:
if local_pid.isdigit():
child_pids.append(int(local_pid))
try:
# Windows does not have SIGKILL, so use SIGTERM instead.
sig = getattr(signal, "SIGKILL", signal.SIGTERM)
os.kill(pid, sig)
os.kill(pid, signal.SIGKILL)
for child_pid in child_pids:
try:
os.kill(child_pid, sig)
os.kill(child_pid, signal.SIGKILL)
except OSError:
pass
kill_check.set() # Tell the main routine that the process was killed.
except OSError:
# It is possible that the process gets completed in the duration after timeout
# happens and before we try to kill the process.
# It is possible that the process gets completed in the duration after
# timeout happens and before we try to kill the process.
pass
return

Expand Down

0 comments on commit 2f017ac

Please sign in to comment.