From c3708bd229c96ef63fdcf1c263c22edc4bf125f7 Mon Sep 17 00:00:00 2001 From: Ran Zvi Date: Wed, 1 Sep 2021 14:59:07 +0300 Subject: [PATCH 1/4] fix: localmode subprocess parent process not sending SIGTERM to child --- setup.py | 1 - src/sagemaker/local/image.py | 7 ++----- src/sagemaker/local/utils.py | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index 2ee10c8971..383a2e2727 100644 --- a/setup.py +++ b/setup.py @@ -52,7 +52,6 @@ def read_version(): "urllib3>=1.21.1,!=1.25,!=1.25.1", "docker-compose>=1.25.2", "PyYAML>=5.3, <6", # PyYAML version has to match docker-compose requirements - "psutil", ], "scipy": ["scipy>=0.19.0"], } diff --git a/src/sagemaker/local/image.py b/src/sagemaker/local/image.py index 573acafeb1..f5afbbd7a0 100644 --- a/src/sagemaker/local/image.py +++ b/src/sagemaker/local/image.py @@ -31,10 +31,7 @@ import tempfile from distutils.spawn import find_executable -from signal import SIGTERM from threading import Thread - -import psutil from six.moves.urllib.parse import urlparse import sagemaker @@ -843,8 +840,8 @@ def run(self): def down(self): """Placeholder docstring""" - for process in psutil.Process(self.process.pid).children(): - process.send_signal(SIGTERM) + if os.name != 'nt': + sagemaker.local.utils.kill_child_processes(self.process.pid) self.process.terminate() diff --git a/src/sagemaker/local/utils.py b/src/sagemaker/local/utils.py index d9455041ab..2db642ee49 100644 --- a/src/sagemaker/local/utils.py +++ b/src/sagemaker/local/utils.py @@ -15,6 +15,7 @@ import os import shutil +import subprocess from distutils.dir_util import copy_tree from six.moves.urllib.parse import urlparse @@ -88,3 +89,24 @@ def recursive_copy(source, destination): """ if os.path.isdir(source): copy_tree(source, destination) + + +def kill_child_processes(pid): + child_pids = get_child_process_ids(pid) + for pid in child_pids: + os.kill(pid, 15) + + +def get_child_process_ids(pid): + cmd = f"pgrep -P {pid}".split() + output, err = subprocess.Popen( + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE + ).communicate() + if err: + return [] + pids = [int(pid) for pid in output.decode('utf-8').split()] + if pids: + for pid in pids: + return pids + get_child_process_ids(pid) + else: + return [] From 8950715f144983801a538d057a3baf12972db700 Mon Sep 17 00:00:00 2001 From: Ran Zvi Date: Sun, 3 Oct 2021 15:00:09 +0300 Subject: [PATCH 2/4] fix: linting --- src/sagemaker/local/image.py | 2 +- src/sagemaker/local/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagemaker/local/image.py b/src/sagemaker/local/image.py index f5afbbd7a0..f0a3ed8579 100644 --- a/src/sagemaker/local/image.py +++ b/src/sagemaker/local/image.py @@ -840,7 +840,7 @@ def run(self): def down(self): """Placeholder docstring""" - if os.name != 'nt': + if os.name != "nt": sagemaker.local.utils.kill_child_processes(self.process.pid) self.process.terminate() diff --git a/src/sagemaker/local/utils.py b/src/sagemaker/local/utils.py index 2db642ee49..0f1511daee 100644 --- a/src/sagemaker/local/utils.py +++ b/src/sagemaker/local/utils.py @@ -104,7 +104,7 @@ def get_child_process_ids(pid): ).communicate() if err: return [] - pids = [int(pid) for pid in output.decode('utf-8').split()] + pids = [int(pid) for pid in output.decode("utf-8").split()] if pids: for pid in pids: return pids + get_child_process_ids(pid) From 177df84809dda1e9cfa12a267b0d51e580ec7137 Mon Sep 17 00:00:00 2001 From: Ran Zvi Date: Mon, 1 Nov 2021 11:26:27 +0200 Subject: [PATCH 3/4] fix: pylint errors --- src/sagemaker/local/utils.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/sagemaker/local/utils.py b/src/sagemaker/local/utils.py index 0f1511daee..7837ba7019 100644 --- a/src/sagemaker/local/utils.py +++ b/src/sagemaker/local/utils.py @@ -92,12 +92,27 @@ def recursive_copy(source, destination): def kill_child_processes(pid): + """Kill child processes + Kills all nested child process ids for a specific pid + + Args: + pid (int): process id + """ child_pids = get_child_process_ids(pid) - for pid in child_pids: - os.kill(pid, 15) + for child_pid in child_pids: + os.kill(child_pid, 15) def get_child_process_ids(pid): + """Retrieve all child pids for a certain pid + Recursively scan each childs process tree and add it to the output + + Args: + pid (int): process id + + Returns: + (List[int]): Child process ids + """ cmd = f"pgrep -P {pid}".split() output, err = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE @@ -106,7 +121,7 @@ def get_child_process_ids(pid): return [] pids = [int(pid) for pid in output.decode("utf-8").split()] if pids: - for pid in pids: - return pids + get_child_process_ids(pid) + for child_pid in pids: + return pids + get_child_process_ids(child_pid) else: return [] From a2ddd4c1d45fdb49226cbf32a09fb1ab0e39cef6 Mon Sep 17 00:00:00 2001 From: Ran Zvi Date: Mon, 1 Nov 2021 11:37:28 +0200 Subject: [PATCH 4/4] fix: pylint errors --- src/sagemaker/local/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sagemaker/local/utils.py b/src/sagemaker/local/utils.py index 7837ba7019..5a8ce03282 100644 --- a/src/sagemaker/local/utils.py +++ b/src/sagemaker/local/utils.py @@ -93,6 +93,7 @@ def recursive_copy(source, destination): def kill_child_processes(pid): """Kill child processes + Kills all nested child process ids for a specific pid Args: @@ -105,6 +106,7 @@ def kill_child_processes(pid): def get_child_process_ids(pid): """Retrieve all child pids for a certain pid + Recursively scan each childs process tree and add it to the output Args: