From 4e1bef3f06073fb67a09f3dcb45e990e4c74f713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Pag=C3=A8s?= Date: Fri, 27 Feb 2015 15:11:14 -0500 Subject: [PATCH] Bug 1124695 - [mozrunner] and/or [mozprocess] should send SIGTERM before sending SIGKILL by default; r=ahal --- .../mozprocess/mozprocess/processhandler.py | 35 ++++++++++++++----- .../mozbase/mozprocess/tests/infinite_loop.py | 18 ++++++++++ .../mozprocess/tests/test_mozprocess_kill.py | 22 ++++++++++++ 3 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 testing/mozbase/mozprocess/tests/infinite_loop.py diff --git a/testing/mozbase/mozprocess/mozprocess/processhandler.py b/testing/mozbase/mozprocess/mozprocess/processhandler.py index a6d2befa12d..8268e67208d 100644 --- a/testing/mozbase/mozprocess/mozprocess/processhandler.py +++ b/testing/mozbase/mozprocess/mozprocess/processhandler.py @@ -65,6 +65,7 @@ class ProcessHandlerMixin(object): MAX_IOCOMPLETION_PORT_NOTIFICATION_DELAY = 180 MAX_PROCESS_KILL_DELAY = 30 + TIMEOUT_BEFORE_SIGKILL = 1.0 def __init__(self, args, @@ -144,16 +145,32 @@ class ProcessHandlerMixin(object): if err is not None: raise OSError(err) else: - sig = sig or signal.SIGKILL - if not self._ignore_children: - try: - os.killpg(self.pid, sig) - except BaseException, e: - if getattr(e, "errno", None) != 3: - # Error 3 is "no such process", which is ok - print >> sys.stdout, "Could not kill process, could not find pid: %s, assuming it's already dead" % self.pid + def send_sig(sig): + if not self._ignore_children: + try: + os.killpg(self.pid, sig) + except BaseException, e: + if getattr(e, "errno", None) != 3: + # Error 3 is "no such process", which is ok + print >> sys.stdout, "Could not kill process, could not find pid: %s, assuming it's already dead" % self.pid + else: + os.kill(self.pid, sig) + + if sig is None and isPosix: + # ask the process for termination and wait a bit + send_sig(signal.SIGTERM) + limit = time.time() + self.TIMEOUT_BEFORE_SIGKILL + while time.time() <= limit: + if self.poll() is not None: + # process terminated nicely + break + time.sleep(0.02) + else: + # process did not terminate - send SIGKILL to force + send_sig(signal.SIGKILL) else: - os.kill(self.pid, sig) + # a signal was explicitly set or not posix + send_sig(sig or signal.SIGKILL) self.returncode = self.wait() self._cleanup() diff --git a/testing/mozbase/mozprocess/tests/infinite_loop.py b/testing/mozbase/mozprocess/tests/infinite_loop.py new file mode 100644 index 00000000000..e38e425e054 --- /dev/null +++ b/testing/mozbase/mozprocess/tests/infinite_loop.py @@ -0,0 +1,18 @@ +import threading +import time +import sys +import signal + +if 'deadlock' in sys.argv: + lock = threading.Lock() + + def trap(sig, frame): + lock.acquire() + + # get the lock once + lock.acquire() + # and take it again on SIGTERM signal: deadlock. + signal.signal(signal.SIGTERM, trap) + +while 1: + time.sleep(1) diff --git a/testing/mozbase/mozprocess/tests/test_mozprocess_kill.py b/testing/mozbase/mozprocess/tests/test_mozprocess_kill.py index 19859f4a989..08261951d60 100644 --- a/testing/mozbase/mozprocess/tests/test_mozprocess_kill.py +++ b/testing/mozbase/mozprocess/tests/test_mozprocess_kill.py @@ -4,6 +4,7 @@ import os import time import unittest import proctest +import signal from mozprocess import processhandler here = os.path.dirname(os.path.abspath(__file__)) @@ -80,5 +81,26 @@ class ProcTestKill(proctest.ProcTest): p.didTimeout, expectedfail=('returncode',)) + @unittest.skipUnless(processhandler.isPosix, "posix only") + def test_process_kill_with_sigterm(self): + script = os.path.join(here, 'infinite_loop.py') + p = processhandler.ProcessHandler([self.python, script]) + + p.run() + p.kill() + + self.assertEquals(p.proc.returncode, -signal.SIGTERM) + + @unittest.skipUnless(processhandler.isPosix, "posix only") + def test_process_kill_with_sigint_if_needed(self): + script = os.path.join(here, 'infinite_loop.py') + p = processhandler.ProcessHandler([self.python, script, 'deadlock']) + + p.run() + time.sleep(1) + p.kill() + + self.assertEquals(p.proc.returncode, -signal.SIGKILL) + if __name__ == '__main__': unittest.main()