utils.process: Avoid waiting for stopped process when timeout set

Previous recursive implementation of waiting for children is not really
possible, because parent has to treat the children exit, otherwise we
get temporary zombie status until the parent process is resumed.

This implementation also uses recursive way of killing the children, but
instead of waiting for them to die it reports the killed processes and
only in the main function it resumes all of them and watches them to
die.

As a benefit people might now get list of killed processes as return.
Signed-off-by: NLukáš Doktor <ldoktor@redhat.com>

avocado/utils/process.py

@@ -212,33 +212,38 @@ def kill_process_tree(pid, sig=signal.SIGKILL, send_sigcont=True,
     :param timeout: How long to wait for the pid(s) to die
                     (negative=infinity, 0=don't wait,
                     positive=number_of_seconds)
+    :return: list of all PIDs we sent signal to
+    :rtype: list
     """
-    if timeout <= 0:
-        def remaining_time():
-            return timeout
-    else:
-        start = time.time()
+    def _all_pids_dead(killed_pids):
+        for pid in killed_pids:
+            if pid_exists(pid):
+                return False
+        return True
 
-        def remaining_time():
-            return timeout - time.time() + start
+    if timeout > 0:
+        start = time.time()
 
     if not safe_kill(pid, signal.SIGSTOP):
-        return
+        return [pid]
+    killed_pids = [pid]
     for child in get_children_pids(pid):
-        kill_process_tree(int(child), sig, timeout=remaining_time())
+        killed_pids.extend(kill_process_tree(int(child), sig, False))
     safe_kill(pid, sig)
     if send_sigcont:
-        safe_kill(pid, signal.SIGCONT)
+        for pid in killed_pids:
+            safe_kill(pid, signal.SIGCONT)
     if timeout == 0:
-        return
+        return killed_pids
     elif timeout > 0:
-        if not wait_for(lambda: not pid_exists(pid), remaining_time(),
-                        step=0.01):
+        if not wait_for(_all_pids_dead, timeout + start - time.time(),
+                        step=0.01, args=(killed_pids[::-1],)):
             raise RuntimeError("Timeout reached when waiting for pid %s "
                                "and children to die (%s)" % (pid, timeout))
     else:
-        while pid_exists(pid):
+        while not _all_pids_dead(killed_pids[::-1]):
             time.sleep(0.01)
+    return killed_pids
 
 
 def kill_process_by_pattern(pattern):

selftests/unit/test_utils_process.py

@@ -454,7 +454,7 @@ class MiscProcessTests(unittest.TestCase):
                                       sleep):
         safe_kill.return_value = True
         get_children_pids.return_value = []
-        process.kill_process_tree(1)
+        self.assertEqual([1], process.kill_process_tree(1))
         self.assertEqual(sleep.call_count, 0)
 
     @mock.patch('avocado.utils.process.safe_kill')
@@ -466,12 +466,13 @@ class MiscProcessTests(unittest.TestCase):
                                           get_children_pids, safe_kill):
         safe_kill.return_value = True
         get_children_pids.return_value = []
-        p_time.side_effect = [500, 502, 504, 506, 508, 510, 512, 514, 516, 518]
+        p_time.side_effect = [500, 502, 502, 502, 502, 502, 502,
+                              504, 504, 504, 520, 520, 520]
         sleep.return_value = None
         pid_exists.return_value = True
-        self.assertRaises(RuntimeError, process.kill_process_tree, 1,
+        self.assertRaises(RuntimeError, process.kill_process_tree, 17,
                           timeout=3)
-        self.assertEqual(p_time.call_count, 5)
+        self.assertLess(p_time.call_count, 10)
 
     @mock.patch('avocado.utils.process.safe_kill')
     @mock.patch('avocado.utils.process.get_children_pids')
@@ -486,7 +487,7 @@ class MiscProcessTests(unittest.TestCase):
         p_time.side_effect = [500, 502, 502, 502, 502, 502, 502, 502, 502, 503]
         sleep.return_value = None
         pid_exists.side_effect = [True, False]
-        process.kill_process_tree(1, timeout=3)
+        self.assertEqual([76], process.kill_process_tree(76, timeout=3))
         self.assertLess(p_time.call_count, 10)
 
     @mock.patch('avocado.utils.process.safe_kill')
@@ -501,11 +502,23 @@ class MiscProcessTests(unittest.TestCase):
         sleep.return_value = None
         pid_exists.side_effect = [True, True, True, True, True, False]
 
-        process.kill_process_tree(1, timeout=-7.354)
+        self.assertEqual([31], process.kill_process_tree(31, timeout=-7.354))
 
         self.assertEqual(pid_exists.call_count, 6)
         self.assertEqual(sleep.call_count, 5)
 
+    @mock.patch('avocado.utils.process.time.sleep')
+    @mock.patch('avocado.utils.process.safe_kill')
+    @mock.patch('avocado.utils.process.get_children_pids')
+    def test_kill_process_tree_children(self, get_children_pids, safe_kill,
+                                        sleep):
+        safe_kill.return_value = True
+        get_children_pids.side_effect = [[53, 12], [78, 58, 41], [], [13],
+                                         [], [], []]
+        self.assertEqual([31, 53, 78, 58, 13, 41, 12],
+                         process.kill_process_tree(31))
+        self.assertEqual(sleep.call_count, 0)
+
 
 class CmdResultTests(unittest.TestCase):