|
Expand Up
|
@@ -13,7 +13,6 @@ |
|
|
import logging |
|
|
import os |
|
|
import re |
|
|
import signal |
|
|
import subprocess |
|
|
from subprocess import DEVNULL, PIPE, Popen |
|
|
import sys |
|
Expand Down
Expand Up
|
@@ -110,7 +109,7 @@ def handle_process_output( |
|
|
stderr_handler: Union[None, Callable[[AnyStr], None], Callable[[List[AnyStr]], None]], |
|
|
finalizer: Union[None, Callable[[Union[Popen, "Git.AutoInterrupt"]], None]] = None, |
|
|
decode_streams: bool = True, |
|
|
kill_after_timeout: Union[None, float] = None, |
|
|
kill_after_timeout: Optional[Union[float, int]] = None, |
|
|
) -> None: |
|
|
R"""Register for notifications to learn that process output is ready to read, and |
|
|
dispatch lines to the respective line handlers. |
|
Expand Down
Expand Up
|
@@ -139,7 +138,7 @@ def handle_process_output( |
|
|
- decoding must happen later, such as for :class:`~git.diff.Diff`\s. |
|
|
|
|
|
:param kill_after_timeout: |
|
|
:class:`float` or ``None``, Default = ``None`` |
|
|
:class:``int``, ``float``, or ``None`` (block indefinitely), Default = ``None``. |
|
|
|
|
|
To specify a timeout in seconds for the git command, after which the process |
|
|
should be killed. |
|
Expand Down
Expand Up
|
@@ -326,16 +325,22 @@ class _AutoInterrupt: |
|
|
raise. |
|
|
""" |
|
|
|
|
|
__slots__ = ("proc", "args", "status") |
|
|
__slots__ = ("proc", "args", "status", "timeout") |
|
|
|
|
|
# If this is non-zero it will override any status code during _terminate, used |
|
|
# to prevent race conditions in testing. |
|
|
_status_code_if_terminate: int = 0 |
|
|
|
|
|
def __init__(self, proc: Union[None, subprocess.Popen], args: Any) -> None: |
|
|
def __init__( |
|
|
self, |
|
|
proc: subprocess.Popen | None, |
|
|
args: Any, |
|
|
timeout: Optional[Union[float, int]] = None, |
|
|
) -> None: |
|
|
self.proc = proc |
|
|
self.args = args |
|
|
self.status: Union[int, None] = None |
|
|
self.timeout = timeout |
Comment thread
Comment on lines
+334
to
+343
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_AutoInterrupt now has a timeout attribute and uses it in wait(timeout=...), but no callers pass a value (the only construction is self.AutoInterrupt(proc, command)). As a result, timeout stays None and the new timeout-aware wait()/_terminate() behavior is never applied. If the intent is to enforce kill_after_timeout for as_process=True flows (e.g., via handle_process_output), wire the timeout through when constructing AutoInterrupt or set it before calling _terminate()/wait().
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Copilot uses AI. Check for mistakes.
|
|
|
|
|
|
def _terminate(self) -> None: |
|
|
"""Terminate the underlying process.""" |
|
Expand Down
Expand Up
|
@@ -365,15 +370,21 @@ def _terminate(self) -> None: |
|
|
# Try to kill it. |
|
|
try: |
|
|
proc.terminate() |
|
|
status = proc.wait() # Ensure the process goes away. |
|
|
status = proc.wait(timeout=self.timeout) # Gracefully wait for the process to terminate. |
|
|
|
|
|
self.status = self._status_code_if_terminate or status |
|
|
except subprocess.TimeoutExpired: |
|
|
_logger.warning("Process did not complete successfully in %s seconds; will forcefully kill", exc_info=True) |
|
|
proc.kill() |
|
|
status = proc.wait() # Ensure the process goes away by blocking with `timeout=None`. |
|
|
self.status = self._status_code_if_terminate or status |
|
|
except (OSError, AttributeError) as ex: |
|
|
# On interpreter shutdown (notably on Windows), parts of the stdlib used by |
|
|
# subprocess can already be torn down (e.g. `subprocess._winapi` becomes None), |
|
|
# which can cause AttributeError during terminate(). In that case, we prefer |
|
|
# to silently ignore to avoid noisy "Exception ignored in: __del__" messages. |
|
|
_logger.info("Ignored error while terminating process: %r", ex) |
Comment thread
Comment on lines
371
to
386
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In _terminate(), if proc.wait(timeout=self.timeout) raises TimeoutExpired, the exception is logged and ignored, potentially leaving the child process running and leaked. Consider escalating to proc.kill() (or another stronger termination strategy) after a terminate+wait timeout, and/or ensuring the process is reaped.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Copilot uses AI. Check for mistakes.
|
|
|
|
|
|
# END exception handling |
|
|
|
|
|
def __del__(self) -> None: |
|
Expand All
|
@@ -383,7 +394,7 @@ def __getattr__(self, attr: str) -> Any: |
|
|
return getattr(self.proc, attr) |
|
|
|
|
|
# TODO: Bad choice to mimic `proc.wait()` but with different args. |
|
|
def wait(self, stderr: Union[None, str, bytes] = b"") -> int: |
|
|
def wait(self, stderr: Optional[Union[str, bytes]] = b"", timeout: Optional[Union[int, float]] = None) -> int: |
|
|
"""Wait for the process and return its status code. |
|
|
|
|
|
:param stderr: |
|
Expand All
|
@@ -400,7 +411,8 @@ def wait(self, stderr: Union[None, str, bytes] = b"") -> int: |
|
|
stderr_b = force_bytes(data=stderr, encoding="utf-8") |
|
|
status: Union[int, None] |
|
|
if self.proc is not None: |
|
|
status = self.proc.wait() |
|
|
timeout = self.timeout if timeout is None else timeout |
|
|
status = self.proc.wait(timeout=timeout) |
|
|
p_stderr = self.proc.stderr |
|
|
else: # Assume the underlying proc was killed earlier or never existed. |
|
|
status = self.status |
|
Expand Down
Expand Up
|
@@ -1106,7 +1118,7 @@ def execute( |
|
|
as_process: bool = False, |
|
|
output_stream: Union[None, BinaryIO] = None, |
|
|
stdout_as_string: bool = True, |
|
|
kill_after_timeout: Union[None, float] = None, |
|
|
kill_after_timeout: Optional[Union[float, int]] = None, |
|
|
with_stdout: bool = True, |
|
|
universal_newlines: bool = False, |
|
|
shell: Union[None, bool] = None, |
|
Expand Down
Expand Up
|
@@ -1156,18 +1168,10 @@ def execute( |
|
|
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. Uses of this feature should be |
|
|
carefully considered, due to the following limitations: |
|
|
carefully considered, due to the following limitation: |
|
|
|
|
|
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 |
|
|
1. 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 |
|
|
:manpage:`git-gc(1)` 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. |
|
Expand Down
Expand Up
|
@@ -1252,15 +1256,7 @@ def execute( |
|
|
if inline_env is not None: |
|
|
env.update(inline_env) |
|
|
|
|
|
if sys.platform == "win32": |
|
|
if kill_after_timeout is not None: |
|
|
raise GitCommandError( |
|
|
redacted_command, |
|
|
'"kill_after_timeout" feature is not supported on Windows.', |
|
|
) |
|
|
cmd_not_found_exception = OSError |
|
|
else: |
|
|
cmd_not_found_exception = FileNotFoundError |
|
|
cmd_not_found_exception = OSError if sys.platform == "win32" else FileNotFoundError |
|
|
# END handle |
|
|
|
|
|
stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb") |
|
Expand Down
Expand Up
|
@@ -1303,66 +1299,14 @@ def execute( |
|
|
if as_process: |
|
|
return self.AutoInterrupt(proc, command) |
|
|
|
|
|
if sys.platform != "win32" and kill_after_timeout is not None: |
|
|
# Help mypy figure out this is not None even when used inside communicate(). |
|
|
timeout = kill_after_timeout |
|
|
|
|
|
def kill_process(pid: int) -> None: |
|
|
"""Callback to kill a process. |
|
|
|
|
|
This callback implementation would be ineffective and unsafe on Windows. |
|
|
""" |
|
|
p = Popen(["ps", "--ppid", str(pid)], stdout=PIPE) |
|
|
child_pids = [] |
|
|
if p.stdout is not None: |
|
|
for line in p.stdout: |
|
|
if len(line.split()) > 0: |
|
|
local_pid = (line.split())[0] |
|
|
if local_pid.isdigit(): |
|
|
child_pids.append(int(local_pid)) |
|
|
try: |
|
|
os.kill(pid, signal.SIGKILL) |
|
|
for child_pid in child_pids: |
|
|
try: |
|
|
os.kill(child_pid, signal.SIGKILL) |
|
|
except OSError: |
|
|
pass |
|
|
# Tell the main routine that the process was killed. |
|
|
kill_check.set() |
|
|
except OSError: |
|
|
# It is possible that the process gets completed in the duration |
|
|
# after timeout happens and before we try to kill the process. |
|
|
pass |
|
|
return |
|
|
|
|
|
def communicate() -> Tuple[AnyStr, AnyStr]: |
|
|
watchdog.start() |
|
|
out, err = proc.communicate() |
|
|
watchdog.cancel() |
|
|
if kill_check.is_set(): |
|
|
err = 'Timeout: the command "%s" did not complete in %d secs.' % ( |
|
|
" ".join(redacted_command), |
|
|
timeout, |
|
|
) |
|
|
if not universal_newlines: |
|
|
err = err.encode(defenc) |
|
|
return out, err |
|
|
|
|
|
# END helpers |
|
|
|
|
|
kill_check = threading.Event() |
|
|
watchdog = threading.Timer(timeout, kill_process, args=(proc.pid,)) |
|
|
else: |
|
|
communicate = proc.communicate |
|
|
|
|
|
# Wait for the process to return. |
|
|
status = 0 |
|
|
stdout_value: Union[str, bytes] = b"" |
|
|
stderr_value: Union[str, bytes] = b"" |
|
|
newline = "\n" if universal_newlines else b"\n" |
|
|
try: |
|
|
if output_stream is None: |
|
|
stdout_value, stderr_value = communicate() |
|
|
stdout_value, stderr_value = proc.communicate(timeout=kill_after_timeout) |
Comment thread
ngie-eign marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
|
|
|
# Strip trailing "\n". |
Comment thread
ngie-eign marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
Comment thread
ngie-eign marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
|
|
|
if stdout_value is not None and stdout_value.endswith(newline) and strip_newline_in_stdout: # type: ignore[arg-type] |
|
|
stdout_value = stdout_value[:-1] |
|
Expand All
|
@@ -1380,8 +1324,18 @@ def communicate() -> Tuple[AnyStr, AnyStr]: |
|
|
# Strip trailing "\n". |
|
|
if stderr_value is not None and stderr_value.endswith(newline): # type: ignore[arg-type] |
|
|
stderr_value = stderr_value[:-1] |
|
|
status = proc.wait() |
|
|
status = proc.wait(timeout=kill_after_timeout) |
|
|
# END stdout handling |
|
|
except subprocess.TimeoutExpired as err: |
|
|
_logger.info( |
|
|
"error: process killed because it timed out. kill_after_timeout=%s seconds", |
|
|
kill_after_timeout, |
|
|
) |
|
|
with contextlib.suppress(subprocess.TimeoutExpired): |
|
|
proc.kill() |
|
|
stdout_value, stderr_value = proc.communicate(timeout=0.1) |
|
|
if with_exceptions: |
|
|
raise GitCommandError(redacted_command, proc.returncode, stderr_value, stdout_value) from err |
|
|
finally: |
|
|
if proc.stdout is not None: |
|
|
proc.stdout.close() |
|
Expand Down
|
|