Skip to content

Commit

Permalink
Use process tree and kill tree instead of parent only. Fix #90 (#91)
Browse files Browse the repository at this point in the history
Bump to 2.1.2
  • Loading branch information
penguinolog committed Oct 23, 2018
1 parent 34ddbd5 commit 027b029
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 29 deletions.
2 changes: 1 addition & 1 deletion exec_helpers/__init__.py
Expand Up @@ -49,7 +49,7 @@
"ExecResult",
)

__version__ = "2.1.1"
__version__ = "2.1.2"
__author__ = "Alexey Stepanov"
__author_email__ = "penguinolog@gmail.com"
__maintainers__ = {
Expand Down
2 changes: 1 addition & 1 deletion exec_helpers/_ssh_client_base.py
Expand Up @@ -116,7 +116,7 @@ def __prepare__( # pylint: disable=unused-argument
.. versionadded:: 1.2.0
"""
return collections.OrderedDict() # pragma: no cover
return collections.OrderedDict()

def __call__( # type: ignore
cls: "_MemorizedSSH",
Expand Down
2 changes: 1 addition & 1 deletion exec_helpers/proc_enums.py
Expand Up @@ -138,7 +138,7 @@ def exit_code_to_enum(code: typing.Union[int, ExitCodes]) -> typing.Union[int, E
"""Convert exit code to enum if possible."""
if isinstance(code, int) and code in ExitCodes.__members__.values():
return ExitCodes(code)
return code
return code # pragma: no cover


def exit_codes_to_enums(
Expand Down
43 changes: 40 additions & 3 deletions exec_helpers/subprocess_runner.py
Expand Up @@ -23,10 +23,12 @@
import concurrent.futures
import errno
import logging
import platform
import subprocess # nosec # Expected usage
import threading
import typing

import psutil # type: ignore
import threaded

from exec_helpers import api
Expand All @@ -37,6 +39,38 @@
logger = logging.getLogger(__name__) # type: logging.Logger


# Adopt from:
# https://stackoverflow.com/questions/1230669/subprocess-deleting-child-processes-in-windows
def kill_proc_tree(pid: int, including_parent: bool = True) -> None: # pragma: no cover
"""Kill process tree.
:param pid: PID of parent process to kill
:type pid: int
:param including_parent: kill also parent process
:type including_parent: bool
"""
parent = psutil.Process(pid)
children = parent.children(recursive=True)
for child in children: # type: psutil.Process
child.kill()
_, alive = psutil.wait_procs(children, timeout=5)
for proc in alive: # type: psutil.Process
proc.kill() # 2nd shot
if including_parent:
parent.kill()
parent.wait(5)


# Subprocess extra arguments.
# Flags from:
# https://stackoverflow.com/questions/13243807/popen-waiting-for-child-process-even-when-the-immediate-child-has-terminated
kw = {} # type: typing.Dict[str, typing.Any]
if "Windows" == platform.system(): # pragma: no cover
kw["creationflags"] = 0x00000200
else: # pragma: no cover
kw["start_new_session"] = True


# noinspection PyTypeHints
class SubprocessExecuteAsyncResult(api.ExecuteAsyncResult):
"""Override original NamedTuple with proper typing."""
Expand Down Expand Up @@ -87,7 +121,7 @@ def __prepare__( # pylint: disable=unused-argument
.. versionadded:: 1.2.0
"""
return collections.OrderedDict() # pragma: no cover
return collections.OrderedDict()


class Subprocess(api.ExecHelper, metaclass=SingletonMeta):
Expand Down Expand Up @@ -172,16 +206,17 @@ def poll_stderr() -> None:
except subprocess.TimeoutExpired:
exit_code = interface.poll() # Update exit code

concurrent.futures.wait([stdout_future, stderr_future], timeout=1) # Minimal timeout to complete polling
concurrent.futures.wait([stdout_future, stderr_future], timeout=0.5) # Minimal timeout to complete polling

# Process closed?
if exit_code is not None:
result.exit_code = exit_code
return result
# Kill not ended process and wait for close
try:
kill_proc_tree(interface.pid, including_parent=False) # kill -9 for all subprocesses
interface.kill() # kill -9
concurrent.futures.wait([stdout_future, stderr_future], timeout=5)
concurrent.futures.wait([stdout_future, stderr_future], timeout=0.5)
# Force stop cycle if no exit code after kill
stdout_future.cancel()
stderr_future.cancel()
Expand Down Expand Up @@ -254,6 +289,7 @@ def execute_async(
cwd=kwargs.get("cwd", None),
env=kwargs.get("env", None),
universal_newlines=False,
**kw
)

if stdin is None:
Expand All @@ -274,6 +310,7 @@ def execute_async(
elif exc.errno in (errno.EPIPE, errno.ESHUTDOWN): # pragma: no cover
self.logger.warning("STDIN Send failed: broken PIPE")
else:
kill_proc_tree(process.pid)
process.kill()
raise
try:
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Expand Up @@ -5,3 +5,4 @@ threaded>=2.0 # Apache-2.0
PyYAML>=3.12 # MIT
advanced-descriptors>=1.0 # Apache-2.0
typing >= 3.6 ; python_version < "3.8"
psutil >= 5.0

0 comments on commit 027b029

Please sign in to comment.