From 2f017ac879eece80e04fd2a30238f261a6b49fb4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 Dec 2023 02:38:05 -0500 Subject: [PATCH] Avoid making it look like kill_process works on Windows 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. --- git/cmd.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index af748e529..27148d3d6 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -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: @@ -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