Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 4a9922f

Browse filesBrowse files
authored
Merge pull request #1761 from EliahKagan/callback-limitations
Clarify some Git.execute kill_after_timeout limitations
2 parents a58a6be + 2f017ac commit 4a9922f
Copy full SHA for 4a9922f

File tree

1 file changed

+20
-16
lines changed
Filter options

1 file changed

+20
-16
lines changed

‎git/cmd.py

Copy file name to clipboardExpand all lines: git/cmd.py
+20-16Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -879,11 +879,19 @@ def execute(
879879
Specifies a timeout in seconds for the git command, after which the process
880880
should be killed. This will have no effect if `as_process` is set to True.
881881
It is set to None by default and will let the process run until the timeout
882-
is explicitly specified. This feature is not supported on Windows. It's also
883-
worth noting that `kill_after_timeout` uses SIGKILL, which can have negative
884-
side effects on a repository. For example, stale locks in case of ``git gc``
885-
could render the repository incapable of accepting changes until the lock is
886-
manually removed.
882+
is explicitly specified. Uses of this feature should be carefully
883+
considered, due to the following limitations:
884+
885+
1. This feature is not supported at all on Windows.
886+
2. Effectiveness may vary by operating system. ``ps --ppid`` is used to
887+
enumerate child processes, which is available on most GNU/Linux systems
888+
but not most others.
889+
3. Deeper descendants do not receive signals, though they may sometimes
890+
terminate as a consequence of their parent processes being killed.
891+
4. `kill_after_timeout` uses ``SIGKILL``, which can have negative side
892+
effects on a repository. For example, stale locks in case of ``git gc``
893+
could render the repository incapable of accepting changes until the lock
894+
is manually removed.
887895
888896
:param with_stdout:
889897
If True, default True, we open stdout on the created process.
@@ -1007,11 +1015,9 @@ def execute(
10071015

10081016
def kill_process(pid: int) -> None:
10091017
"""Callback to kill a process."""
1010-
p = Popen(
1011-
["ps", "--ppid", str(pid)],
1012-
stdout=PIPE,
1013-
creationflags=PROC_CREATIONFLAGS,
1014-
)
1018+
if os.name == "nt":
1019+
raise AssertionError("Bug: This callback would be ineffective and unsafe on Windows, stopping.")
1020+
p = Popen(["ps", "--ppid", str(pid)], stdout=PIPE)
10151021
child_pids = []
10161022
if p.stdout is not None:
10171023
for line in p.stdout:
@@ -1020,18 +1026,16 @@ def kill_process(pid: int) -> None:
10201026
if local_pid.isdigit():
10211027
child_pids.append(int(local_pid))
10221028
try:
1023-
# Windows does not have SIGKILL, so use SIGTERM instead.
1024-
sig = getattr(signal, "SIGKILL", signal.SIGTERM)
1025-
os.kill(pid, sig)
1029+
os.kill(pid, signal.SIGKILL)
10261030
for child_pid in child_pids:
10271031
try:
1028-
os.kill(child_pid, sig)
1032+
os.kill(child_pid, signal.SIGKILL)
10291033
except OSError:
10301034
pass
10311035
kill_check.set() # Tell the main routine that the process was killed.
10321036
except OSError:
1033-
# It is possible that the process gets completed in the duration after timeout
1034-
# happens and before we try to kill the process.
1037+
# It is possible that the process gets completed in the duration after
1038+
# timeout happens and before we try to kill the process.
10351039
pass
10361040
return
10371041

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.