change preexec_fn usage in Popen to start_new_session=True instead

Author: arjpur01

devlib/connection.py

@@ -424,6 +424,21 @@ def __enter__(self):
         self.popen.__enter__()
         return self
 
+    def __exit__(self, *args, **kwargs):
+        # Send SIGINT to the process group to allow it to clean up if it is running
+        if self.popen and self.popen.poll() is None:
+            try:
+                os.killpg(self.popen.pid, signal.SIGINT)
+            except Exception:
+                pass
+            try:
+                # allow a graceful termination for 60 seconds
+                self.popen.wait(timeout=60)
+            except subprocess.TimeoutExpired:
+                # If the process did not terminate, send SIGKILL
+                os.killpg(self.popen.pid, signal.SIGKILL)
+                self.popen.wait()
+
 
 class ParamikoBackgroundCommand(BackgroundCommand):
     """
@@ -638,6 +653,21 @@ def __enter__(self):
         self.adb_popen.__enter__()
         return self
 
+    def __exit__(self, *args, **kwargs):
+        # Send SIGINT to the process group to allow it to clean up if it is running
+        if self.adb_popen.poll() is None:
+            try:
+                os.killpg(self.adb_popen.pid, signal.SIGINT)
+            except Exception:
+                pass
+            try:
+                # allow a graceful termination for 60 seconds
+                self.adb_popen.wait(timeout=60)
+            except subprocess.TimeoutExpired:
+                # If the process did not terminate, send SIGKILL
+                os.killpg(self.adb_popen.pid, signal.SIGKILL)
+                self.adb_popen.wait()
+
 
 class TransferManager:
     def __init__(self, conn, transfer_poll_period=30, start_transfer_poll_delay=30, total_transfer_timeout=3600):

devlib/host.py

@@ -144,8 +144,6 @@ def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as
 
         # Make sure to get a new PGID so PopenBackgroundCommand() can kill
         # all sub processes that could be started without troubles.
-        def preexec_fn():
-            os.setpgrp()
 
         def make_init_kwargs(command):
             popen = subprocess.Popen(
@@ -154,7 +152,7 @@ def make_init_kwargs(command):
                 stderr=stderr,
                 stdin=subprocess.PIPE,
                 shell=True,
-                preexec_fn=preexec_fn,
+                start_new_session=True,
             )
             return dict(
                 popen=popen,

devlib/instrument/arm_energy_probe.py

@@ -102,6 +102,9 @@ def reset(self, sites=None, kinds=None, channels=None):
 
     def start(self):
         self.logger.debug(self.command)
+        # FIXME - replace this preexec_fn arg with start_new_session argument.
+        # to address https://github.com/ARM-software/devlib/issues/708.
+        # didnt do it with initial change for the fix due to lack of test hardware
         self.armprobe = subprocess.Popen(self.command,
                                        stderr=self.output_fd_error,
                                        preexec_fn=os.setpgrp,

devlib/instrument/energy_probe.py

@@ -75,6 +75,9 @@ def reset(self, sites=None, kinds=None, channels=None):
 
     def start(self):
         self.logger.debug(self.command)
+        # FIXME - replace this preexec_fn arg with start_new_session argument.
+        # to address https://github.com/ARM-software/devlib/issues/708.
+        # didnt do it with initial change for the fix due to lack of test hardware
         self.process = subprocess.Popen(self.command,
                                         stdout=subprocess.PIPE,
                                         stderr=subprocess.PIPE,

devlib/utils/android.py

@@ -687,7 +687,8 @@ def with_uuid(cmd):
     adb_cmd = get_adb_command(device, 'shell', adb_server, adb_port)
     full_command = f'{adb_cmd} {quote(command)}'
     logger.debug(full_command)
-    p = subprocess.Popen(full_command, stdout=stdout, stderr=stderr, stdin=subprocess.PIPE, shell=True)
+    p = subprocess.Popen(full_command, stdout=stdout, stderr=stderr, stdin=subprocess.PIPE, shell=True,
+                         start_new_session=True)
 
     # Out of band PID lookup, to avoid conflicting needs with stdout redirection
     grep_cmd = f'{busybox} grep {quote(command_uuid)}'

devlib/utils/misc.py

@@ -136,14 +136,6 @@ def get_cpu_name(implementer, part, variant):
     return name
 
 
-def preexec_function():
-    # Change process group in case we have to kill the subprocess and all of
-    # its children later.
-    # TODO: this is Unix-specific; would be good to find an OS-agnostic way
-    #       to do this in case we wanna port WA to Windows.
-    os.setpgrp()
-
-
 check_output_logger = logging.getLogger('check_output')
 
 def get_subprocess(command, **kwargs):
@@ -153,7 +145,7 @@ def get_subprocess(command, **kwargs):
                                 stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE,
                                 stdin=subprocess.PIPE,
-                                preexec_fn=preexec_function,
+                                start_new_session=True,
                                 **kwargs)