From 4e1eb97056001d1da4f23dc3cfe63e671f15590b Mon Sep 17 00:00:00 2001
From: wevsty <11432483+wevsty@users.noreply.github.com>
Date: Mon, 13 Jul 2020 20:31:14 +0800
Subject: [PATCH] Split kill_house() fix issues #1234 (#1300)

* Split kill_house() fix issues #1234

Details View issues #1234

* Removing the catch finally

* rename wait_horse() to wait_for_horse()

* rename wait_horse() to wait_for_horse()

* update test_handle_shutdown_request()

Change test_handle_shutdown_request() exitcode assert

* Restore kill_horse() output

* optimization wait_for_horse()
---
 rq/worker.py         | 18 ++++++++++++++++--
 tests/test_worker.py |  5 ++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/rq/worker.py b/rq/worker.py
index 87d405f..7f443ce 100644
--- a/rq/worker.py
+++ b/rq/worker.py
@@ -397,7 +397,6 @@ class Worker(object):
         """
         try:
             os.kill(self.horse_pid, sig)
-            os.waitpid(self.horse_pid, 0)
             self.log.info('Killed horse pid %s', self.horse_pid)
         except OSError as e:
             if e.errno == errno.ESRCH:
@@ -406,6 +405,19 @@ class Worker(object):
             else:
                 raise
 
+    def wait_for_horse(self):
+        """
+        A waiting the end of the horse process and recycling resources.
+        """
+        pid = None
+        stat = None
+        try:
+            pid, stat = os.waitpid(self.horse_pid, 0)
+        except ChildProcessError as e:
+            # ChildProcessError: [Errno 10] No child processes
+            pass
+        return pid, stat
+
     def request_force_stop(self, signum, frame):
         """Terminates the application (cold shutdown).
         """
@@ -415,6 +427,7 @@ class Worker(object):
         if self.horse_pid:
             self.log.debug('Taking down horse %s with me', self.horse_pid)
             self.kill_horse()
+            self.wait_for_horse()
         raise SystemExit()
 
     def request_stop(self, signum, frame):
@@ -704,7 +717,7 @@ class Worker(object):
         while True:
             try:
                 with UnixSignalDeathPenalty(self.job_monitoring_interval, HorseMonitorTimeoutException):
-                    retpid, ret_val = os.waitpid(self._horse_pid, 0)
+                    retpid, ret_val = self.wait_for_horse()
                 break
             except HorseMonitorTimeoutException:
                 # Horse has not exited yet and is still running.
@@ -714,6 +727,7 @@ class Worker(object):
                 # Kill the job from this side if something is really wrong (interpreter lock/etc).
                 if job.timeout != -1 and (utcnow() - job.started_at).total_seconds() > (job.timeout + 60):
                     self.kill_horse()
+                    self.wait_for_horse()
                     break
 
             except OSError as e:
diff --git a/tests/test_worker.py b/tests/test_worker.py
index 8e8564f..df902ac 100644
--- a/tests/test_worker.py
+++ b/tests/test_worker.py
@@ -1212,9 +1212,8 @@ class HerokuWorkerShutdownTestCase(TimeoutTestCase, RQTestCase):
         w._horse_pid = p.pid
         w.handle_warm_shutdown_request()
         p.join(2)
-        # would expect p.exitcode to be -34 but for some reason os.waitpid is setting it to None, even though
-        # the process has ended
-        self.assertEqual(p.exitcode, None)
+        # would expect p.exitcode to be -34
+        self.assertEqual(p.exitcode, -34)
         self.assertFalse(os.path.exists(path))
         mock_logger_info.assert_called_with('Killed horse pid %s', p.pid)