Content-Length: 5077 | pFad | http://github.com/gitpython-developers/GitPython/pull/1761.patch

thub.com From f42a63bf54f63fa7e266331906d3939a98c3696c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 Dec 2023 02:37:11 -0500 Subject: [PATCH 1/2] Document more Git.execute kill_after_timeout limitations See discussion in #1756. --- git/cmd.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 8b42dec52..af748e529 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -879,11 +879,19 @@ def execute( Specifies a timeout in seconds for the git command, after which the process should be killed. This will have no effect if `as_process` is set to True. It is set to None by default and will let the process run until the timeout - is explicitly specified. This feature is not supported on Windows. It's also - worth noting that `kill_after_timeout` uses SIGKILL, which can have negative - side effects on a repository. For example, stale locks in case of ``git gc`` - could render the repository incapable of accepting changes until the lock is - manually removed. + is explicitly specified. Uses of this feature should be carefully + considered, due to the following limitations: + + 1. This feature is not supported at all on Windows. + 2. Effectiveness may vary by operating system. ``ps --ppid`` is used to + enumerate child processes, which is available on most GNU/Linux systems + but not most others. + 3. Deeper descendants do not receive signals, though they may sometimes + terminate as a consequence of their parent processes being killed. + 4. `kill_after_timeout` uses ``SIGKILL``, which can have negative side + effects on a repository. For example, stale locks in case of ``git gc`` + could render the repository incapable of accepting changes until the lock + is manually removed. :param with_stdout: If True, default True, we open stdout on the created process. From 2f017ac879eece80e04fd2a30238f261a6b49fb4 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 10 Dec 2023 02:38:05 -0500 Subject: [PATCH 2/2] 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

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!