From 772bd64be00826be385c2735ededef3fd31dd894 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Wed, 19 Mar 2014 17:26:00 +0100 Subject: [PATCH 01/22] Add signals module with register() function #7 --- pkit/signals/__init__.py | 1 + pkit/signals/base.py | 31 +++++++++++++++++++++++ setup.py | 4 ++- tests/signals/test_base.py | 52 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 pkit/signals/__init__.py create mode 100644 pkit/signals/base.py create mode 100644 tests/signals/test_base.py diff --git a/pkit/signals/__init__.py b/pkit/signals/__init__.py new file mode 100644 index 0000000..f7a77b2 --- /dev/null +++ b/pkit/signals/__init__.py @@ -0,0 +1 @@ +from base import * diff --git a/pkit/signals/base.py b/pkit/signals/base.py new file mode 100644 index 0000000..95c474a --- /dev/null +++ b/pkit/signals/base.py @@ -0,0 +1,31 @@ +import signal +import collections + + +__all__ = ['register'] + + +SIGNAL_NUMBERS = {v for k, v in vars(signal).iteritems() if + k.startswith('SIG')} +SIGNAL_HANDLERS = collections.defaultdict(list) + + +def call_signal_handler(signum): + def handle_signal(*args, **kwargs): + for handler in SIGNAL_HANDLERS[signum]: + handler(*args, **kwargs) + + return handle_signal + + +def register(signum, handler): + if signum not in SIGNAL_NUMBERS: + raise ValueError('Unknow signal number {}'.format(signum)) + + if not callable(handler): + raise TypeError('handler must be callable') + + if signum not in SIGNAL_HANDLERS: + signal.signal(signum, call_signal_handler(signum)) + + SIGNAL_HANDLERS[signum].append(handler) diff --git a/setup.py b/setup.py index 3cbf642..652fd90 100644 --- a/setup.py +++ b/setup.py @@ -22,6 +22,8 @@ packages=[ 'pkit', - 'pkit.slot' + 'pkit.slot', + + 'pkit.signals', ], ) diff --git a/tests/signals/test_base.py b/tests/signals/test_base.py new file mode 100644 index 0000000..551ac84 --- /dev/null +++ b/tests/signals/test_base.py @@ -0,0 +1,52 @@ +import unittest +import collections + +from pkit.signals import base + + +class TestBase(unittest.TestCase): + def setUp(self): + base.SIGNAL_HANDLERS = collections.defaultdict(list) + + +class TestCallSignalHandler(TestBase): + def test_call_signal_handler(self): + result = [None] + + def handler(string, *args): + result[0] = '{} works.'.format(string) + + base.SIGNAL_HANDLERS[base.signal.SIGTERM] = [handler] + base.call_signal_handler(base.signal.SIGTERM)('test') + self.assertEquals(result[0], 'test works.') + + +class TestRegister(TestBase): + def test_register_unknown_signum_raises(self): + with self.assertRaises(ValueError): + base.register('aint no signal', lambda: None) + + def test_register_handler_with_invalid_type(self): + with self.assertRaises(TypeError): + base.register(base.signal.SIGTERM, 'not a function') + + def test_register_one_handler(self): + def handler(*args): + return + + base.register(base.signal.SIGTERM, handler) + self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], + [handler]) + + def test_register_two_handlers(self): + def handler1(*args): + return + + def handler2(*args): + return + + base.register(base.signal.SIGTERM, handler1) + base.register(base.signal.SIGTERM, handler2) + + self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], + [handler1, handler2]) From 292a96c8436969ce5f596a5bb9a6b2054fec94fc Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Wed, 19 Mar 2014 17:45:57 +0100 Subject: [PATCH 02/22] Update signals: add unregister() #7 --- pkit/signals/base.py | 15 +++++++++++++- tests/signals/test_base.py | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pkit/signals/base.py b/pkit/signals/base.py index 95c474a..5db5703 100644 --- a/pkit/signals/base.py +++ b/pkit/signals/base.py @@ -2,7 +2,7 @@ import collections -__all__ = ['register'] +__all__ = ['register', 'unregister'] SIGNAL_NUMBERS = {v for k, v in vars(signal).iteritems() if @@ -29,3 +29,16 @@ def register(signum, handler): signal.signal(signum, call_signal_handler(signum)) SIGNAL_HANDLERS[signum].append(handler) + + +def unregister(signum, handler): + if signum not in SIGNAL_HANDLERS: + raise LookupError('signal number {} not found'.format(signum)) + + handlers = SIGNAL_HANDLERS[signum] + try: + handlers.remove(handler) + except ValueError: + raise LookupError('handler {} not found for signal number {}'.format( + handler, + signum)) diff --git a/tests/signals/test_base.py b/tests/signals/test_base.py index 551ac84..419aa13 100644 --- a/tests/signals/test_base.py +++ b/tests/signals/test_base.py @@ -50,3 +50,45 @@ def handler2(*args): self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], [handler1, handler2]) + + +class TestUnregister(unittest.TestCase): + def test_unregister_one_handler(self): + def handler(*args): + return + + base.register(base.signal.SIGTERM, handler) + base.unregister(base.signal.SIGTERM, handler) + self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], + []) + + def test_unregister_two_handlers(self): + def handler1(*args): + return + + def handler2(*args): + return + + base.register(base.signal.SIGTERM, handler1) + base.register(base.signal.SIGTERM, handler2) + + base.unregister(base.signal.SIGTERM, handler1) + self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], + [handler2]) + + base.unregister(base.signal.SIGTERM, handler2) + self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], + []) + + def test_unregister_signal_not_found(self): + with self.assertRaises(LookupError): + base.unregister(base.signal.SIGTERM, 'test') + + def test_unregister_handler_not_found(self): + def handler(*args): + return + + base.register(base.signal.SIGTERM, handler) + + with self.assertRaises(LookupError): + base.unregister(base.signal.SIGTERM, 'test') From 3f93f91a2f9c74bd6f3f4bc95ed245cf99772038 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Wed, 19 Mar 2014 17:52:35 +0100 Subject: [PATCH 03/22] Update process #7: use signals.register() in ProcessOpen --- pkit/process.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkit/process.py b/pkit/process.py index ae3d6a8..3f1e8bc 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -7,6 +7,9 @@ import traceback +from pkit import signals + + JOIN_RESTART_POLICY = 0 TERMINATE_RESTART_POLICY = 1 @@ -54,7 +57,8 @@ def __init__(self, process, wait=False, wait_timeout=1): self.pid = os.fork() if self.pid == 0: - signal.signal(signal.SIGTERM, self.on_sigterm) + signals.register(signal.SIGTERM, self.on_sigterm) + # Once the child process has it's signal handler # binded we warn the parent process through a pipe if wait is True: From fc00a8b00f4167b0fc68db7e111e56922e0d35b3 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Wed, 19 Mar 2014 18:10:58 +0100 Subject: [PATCH 04/22] Update signals #7: move signal constants into signals.constants --- pkit/signals/base.py | 6 +++--- pkit/signals/constants.py | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 pkit/signals/constants.py diff --git a/pkit/signals/base.py b/pkit/signals/base.py index 5db5703..70de176 100644 --- a/pkit/signals/base.py +++ b/pkit/signals/base.py @@ -1,12 +1,12 @@ import signal import collections +from pkit.signals import constants + __all__ = ['register', 'unregister'] -SIGNAL_NUMBERS = {v for k, v in vars(signal).iteritems() if - k.startswith('SIG')} SIGNAL_HANDLERS = collections.defaultdict(list) @@ -19,7 +19,7 @@ def handle_signal(*args, **kwargs): def register(signum, handler): - if signum not in SIGNAL_NUMBERS: + if signum not in constants.SIGNAL_NUMBERS: raise ValueError('Unknow signal number {}'.format(signum)) if not callable(handler): diff --git a/pkit/signals/constants.py b/pkit/signals/constants.py new file mode 100644 index 0000000..3e4e108 --- /dev/null +++ b/pkit/signals/constants.py @@ -0,0 +1,21 @@ +import sys +import signal + + +__all__ = [] # Fill below + + +current_module = sys.modules[__name__] + +SIGNALS = {k: v for k, v in vars(signal).iteritems() if + k.startswith('SIG')} + +SIGNAL_NUMBERS = SIGNALS.values() + + +def set_signals(): + for signame, signum in SIGNALS.iteritems(): + setattr(current_module, signame, signum) + __all__.append(signame) + +set_signals() From d88e1b789316dde19b527a6b0b3a9ce043a4aa50 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Fri, 21 Mar 2014 18:58:56 +0100 Subject: [PATCH 05/22] Update signals: update the signature of handler --- pkit/signals/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkit/signals/base.py b/pkit/signals/base.py index 70de176..aa01954 100644 --- a/pkit/signals/base.py +++ b/pkit/signals/base.py @@ -11,9 +11,9 @@ def call_signal_handler(signum): - def handle_signal(*args, **kwargs): + def handle_signal(signum, sigframe): for handler in SIGNAL_HANDLERS[signum]: - handler(*args, **kwargs) + handler(signum, sigframe) return handle_signal From 89342960a88f0924fbda618d62a5ce6bf08d0c2b Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Fri, 21 Mar 2014 19:04:26 +0100 Subject: [PATCH 06/22] Add module signals.registry --- pkit/signals/registry.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 pkit/signals/registry.py diff --git a/pkit/signals/registry.py b/pkit/signals/registry.py new file mode 100644 index 0000000..87b4f04 --- /dev/null +++ b/pkit/signals/registry.py @@ -0,0 +1,31 @@ +import collections + + +class HandlerRegistry(object): + def __init__(self, extract_from, insert_with): + """ + :param extract_from: returns the key of the object to find when a + signal is triggered. + :type extract_from: callable(signum, sigframe) + + :param insert_with: returns the value of the key used to index the + object in the registry. + :type insert_with: callable(obj) + + """ + self._extract_from = extract_from + self._insert_with = insert_with + self._handlers = collections.defaultdict(list) + + def register(self, obj, handler): + key = self._insert_with(obj) + self._handlers[key].append(handler) + + def unregister(self, obj, handler): + key = self._insert_with(obj) + self._handlers[key].remove(handler) + + def __call__(self, signum, sigframe): + key = self._extract_from(signum, sigframe) + for handler in self._handlers[key]: + handler() From dadd536f86dde6398668ad9ac81e8bba7d31f7bc Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Fri, 21 Mar 2014 19:04:50 +0100 Subject: [PATCH 07/22] Add module signals.process #7: dispatch a signal to the PID of a process that just exited --- pkit/signals/__init__.py | 5 ++- pkit/signals/process.py | 20 +++++++++++ tests/signals/test_process.py | 62 +++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 pkit/signals/process.py create mode 100644 tests/signals/test_process.py diff --git a/pkit/signals/__init__.py b/pkit/signals/__init__.py index f7a77b2..33fdf78 100644 --- a/pkit/signals/__init__.py +++ b/pkit/signals/__init__.py @@ -1 +1,4 @@ -from base import * +from __future__ import absolute_import + +from .base import * +from . import process diff --git a/pkit/signals/process.py b/pkit/signals/process.py new file mode 100644 index 0000000..c9d77c3 --- /dev/null +++ b/pkit/signals/process.py @@ -0,0 +1,20 @@ +import os + +import pkit.signals +from pkit.signals.registry import HandlerRegistry + + +def register(signum, obj, handler): + registry_handler = None + for i in pkit.signals.base.SIGNAL_HANDLERS.values(): + if isinstance(i, HandlerRegistry): + registry_handler = i + break + + if registry_handler is None: + registry_handler = HandlerRegistry( + extract_from=lambda *_: os.waitpid()[0], + insert_with=lambda process: process.pid) + pkit.signals.base.register(signum, registry_handler) + + registry_handler.register(obj, handler) diff --git a/tests/signals/test_process.py b/tests/signals/test_process.py new file mode 100644 index 0000000..b8f4120 --- /dev/null +++ b/tests/signals/test_process.py @@ -0,0 +1,62 @@ +import mock + +import pkit.process +from pkit import signals + +from pkit.signals.base import call_signal_handler + + +def test_register_process(): + SIGCHLD = signals.constants.SIGCHLD + pid = 42 + process = pkit.process.Process(target=lambda: None) + process._child = lambda: None + process._child.pid = pid + + vals = [] + process.on_sigchld = lambda *_: vals.append('OK') + signals.process.register( + SIGCHLD, + process, + process.on_sigchld) + + with mock.patch('os.waitpid', return_value=(pid, 0)): + call_signal_handler(SIGCHLD)(SIGCHLD, None) + + assert vals == ['OK'] + + +def test_register_two_process(): + SIGCHLD = signals.constants.SIGCHLD + pid1 = 42 + pid2 = 1234 + + process1 = pkit.process.Process(target=lambda: None) + process1._child = lambda: None + process1._child.pid = pid1 + + process2 = pkit.process.Process(target=lambda: None) + process2._child = lambda: None + process2._child.pid = pid2 + + vals = [] + + process1.on_sigchld = lambda *_: vals.append(1) + signals.process.register( + SIGCHLD, + process1, + process1.on_sigchld) + + process2.on_sigchld = lambda *_: vals.append(2) + signals.process.register( + SIGCHLD, + process2, + process2.on_sigchld) + + with mock.patch('os.waitpid', return_value=(pid1, 0)): + call_signal_handler(SIGCHLD)(SIGCHLD, None) + + with mock.patch('os.waitpid', return_value=(pid2, 0)): + call_signal_handler(SIGCHLD)(SIGCHLD, None) + + assert vals == (1, 2) From ab41dc26719ae35ce213592a74e29b5210fe505c Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:45:50 +0200 Subject: [PATCH 08/22] Update pool: check if process.pit is None in on_process_exit() --- pkit/pool.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkit/pool.py b/pkit/pool.py index 0ce63b7..c2726d1 100644 --- a/pkit/pool.py +++ b/pkit/pool.py @@ -132,6 +132,9 @@ def terminate(self, wait=False): task.process.terminate(wait=wait) def on_process_exit(self, process): + if process.pid is None: # Already handled. + return + self.slots.release() pid = process.pid From f026e7298a27c3a157c3a07db586a366b5650363 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:47:09 +0200 Subject: [PATCH 09/22] Add signals.base.reset(): empty SIGNAL_HANDLERS --- pkit/signals/base.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkit/signals/base.py b/pkit/signals/base.py index aa01954..785e2e5 100644 --- a/pkit/signals/base.py +++ b/pkit/signals/base.py @@ -10,6 +10,10 @@ SIGNAL_HANDLERS = collections.defaultdict(list) +def reset(): + SIGNAL_HANDLERS = collections.defaultdict(list) + + def call_signal_handler(signum): def handle_signal(signum, sigframe): for handler in SIGNAL_HANDLERS[signum]: From cc254c8bc4580a567d33f435871122306362834d Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:48:10 +0200 Subject: [PATCH 10/22] Fix signals.base: avoid signum key with empty list of handlers --- pkit/signals/base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkit/signals/base.py b/pkit/signals/base.py index 785e2e5..de81d1c 100644 --- a/pkit/signals/base.py +++ b/pkit/signals/base.py @@ -16,6 +16,9 @@ def reset(): def call_signal_handler(signum): def handle_signal(signum, sigframe): + if signum not in SIGNAL_HANDLERS: + return None + for handler in SIGNAL_HANDLERS[signum]: handler(signum, sigframe) @@ -29,7 +32,7 @@ def register(signum, handler): if not callable(handler): raise TypeError('handler must be callable') - if signum not in SIGNAL_HANDLERS: + if signum not in SIGNAL_HANDLERS or not SIGNAL_HANDLERS[signum]: signal.signal(signum, call_signal_handler(signum)) SIGNAL_HANDLERS[signum].append(handler) From 89bec8f7e59a95a5c8b5867a6cae57b22579c5c1 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:49:17 +0200 Subject: [PATCH 11/22] Fix signals.process --- pkit/signals/process.py | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/pkit/signals/process.py b/pkit/signals/process.py index c9d77c3..02959c6 100644 --- a/pkit/signals/process.py +++ b/pkit/signals/process.py @@ -1,20 +1,49 @@ import os import pkit.signals +from pkit.signals.base import SIGNAL_HANDLERS from pkit.signals.registry import HandlerRegistry -def register(signum, obj, handler): - registry_handler = None - for i in pkit.signals.base.SIGNAL_HANDLERS.values(): +def get_last_exited_pid(): + try: + pid, _ = os.wait() + except OSError: + return None + + return pid + + +def get_registry_handler(signum, handlers=SIGNAL_HANDLERS): + if signum not in handlers: + return None + + for i in handlers[signum]: if isinstance(i, HandlerRegistry): registry_handler = i break + return registry_handler + + +def register(signum, process, handler): + registry_handler = get_registry_handler(signum) + if registry_handler is None: registry_handler = HandlerRegistry( - extract_from=lambda *_: os.waitpid()[0], + extract_from=lambda *_: get_last_exited_pid(), insert_with=lambda process: process.pid) pkit.signals.base.register(signum, registry_handler) - registry_handler.register(obj, handler) + registry_handler.register(process, handler) + + +def unregister(signum, process, handler): + registry_handler = get_registry_handler(signum) + + if registry_handler is None: + raise LookupError( + 'Handler {} for process {} and signal {} not found'.format( + handler, process, signum)) + + registry_handler.unregister(process, handler) From f0208c1988e5b8a20807378ea3ce4787633f82c3 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:49:47 +0200 Subject: [PATCH 12/22] Fix signals.registry --- pkit/signals/registry.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkit/signals/registry.py b/pkit/signals/registry.py index 87b4f04..d05a5ec 100644 --- a/pkit/signals/registry.py +++ b/pkit/signals/registry.py @@ -27,5 +27,8 @@ def unregister(self, obj, handler): def __call__(self, signum, sigframe): key = self._extract_from(signum, sigframe) + if key not in self._handlers: + return + for handler in self._handlers[key]: - handler() + handler(signum, sigframe) From 4ad8a7ca99dfac945b794371696b043a493ac1bd Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:50:07 +0200 Subject: [PATCH 13/22] Update utils.wait() --- pkit/utils.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkit/utils.py b/pkit/utils.py index dba0c70..548cf34 100644 --- a/pkit/utils.py +++ b/pkit/utils.py @@ -3,16 +3,20 @@ from pkit.exceptions import TimeoutError +DEFAULT_INTERVAL = 0.005 + + def wait(until=None, timeout=None, args=(), kwargs={}): if not callable(until): raise TypeError("until must be callable") elapsed = 0.0 + interval = DEFAULT_INTERVAL while until(*args, **kwargs) is False: - time.sleep(0.005) - elapsed += 0.005 - + time.sleep(interval) + elapsed += interval + if timeout is not None and elapsed >= timeout: raise TimeoutError From c1f147ccef47d4f8991378f969cbb08c352f666b Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:50:47 +0200 Subject: [PATCH 14/22] Fix tests.signals.test_base: move to pytest --- tests/signals/test_base.py | 77 ++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/tests/signals/test_base.py b/tests/signals/test_base.py index 419aa13..6705507 100644 --- a/tests/signals/test_base.py +++ b/tests/signals/test_base.py @@ -1,11 +1,15 @@ -import unittest +import pytest import collections from pkit.signals import base +from pkit.signals import constants -class TestBase(unittest.TestCase): - def setUp(self): +class TestBase(object): + def setup_method(self): + base.SIGNAL_HANDLERS = collections.defaultdict(list) + + def teardown_method(self): base.SIGNAL_HANDLERS = collections.defaultdict(list) @@ -13,30 +17,31 @@ class TestCallSignalHandler(TestBase): def test_call_signal_handler(self): result = [None] - def handler(string, *args): - result[0] = '{} works.'.format(string) + def handler(signum, sigframe): + result[0] = '{} works.'.format(signum) + + base.SIGNAL_HANDLERS[constants.SIGTERM] = [handler] + base.call_signal_handler(constants.SIGTERM)( - base.SIGNAL_HANDLERS[base.signal.SIGTERM] = [handler] - base.call_signal_handler(base.signal.SIGTERM)('test') - self.assertEquals(result[0], 'test works.') + constants.SIGTERM, None) + result[0] == '{} works.'.format(constants.SIGTERM) class TestRegister(TestBase): def test_register_unknown_signum_raises(self): - with self.assertRaises(ValueError): + with pytest.raises(ValueError): base.register('aint no signal', lambda: None) def test_register_handler_with_invalid_type(self): - with self.assertRaises(TypeError): - base.register(base.signal.SIGTERM, 'not a function') + with pytest.raises(TypeError): + base.register(constants.SIGTERM, 'not a function') def test_register_one_handler(self): def handler(*args): return - base.register(base.signal.SIGTERM, handler) - self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], - [handler]) + base.register(constants.SIGTERM, handler) + base.SIGNAL_HANDLERS[constants.SIGTERM] == [handler] def test_register_two_handlers(self): def handler1(*args): @@ -45,22 +50,24 @@ def handler1(*args): def handler2(*args): return - base.register(base.signal.SIGTERM, handler1) - base.register(base.signal.SIGTERM, handler2) + base.register(constants.SIGTERM, handler1) + base.register(constants.SIGTERM, handler2) - self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], - [handler1, handler2]) + base.SIGNAL_HANDLERS[constants.SIGTERM] = [handler1, handler2] -class TestUnregister(unittest.TestCase): +class TestUnregister(object): + def setup_module(self): + base.SIGNAL_HANDLERS = collections.defaultdict(list) + def test_unregister_one_handler(self): def handler(*args): return - base.register(base.signal.SIGTERM, handler) - base.unregister(base.signal.SIGTERM, handler) - self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], - []) + base.register(constants.SIGTERM, handler) + + base.unregister(constants.SIGTERM, handler) + base.SIGNAL_HANDLERS[constants.SIGTERM] == [] def test_unregister_two_handlers(self): def handler1(*args): @@ -69,26 +76,24 @@ def handler1(*args): def handler2(*args): return - base.register(base.signal.SIGTERM, handler1) - base.register(base.signal.SIGTERM, handler2) + base.register(constants.SIGTERM, handler1) + base.register(constants.SIGTERM, handler2) - base.unregister(base.signal.SIGTERM, handler1) - self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], - [handler2]) + base.unregister(constants.SIGTERM, handler1) + base.SIGNAL_HANDLERS[constants.SIGTERM] == [handler2] - base.unregister(base.signal.SIGTERM, handler2) - self.assertEquals(base.SIGNAL_HANDLERS[base.signal.SIGTERM], - []) + base.unregister(constants.SIGTERM, handler2) + base.SIGNAL_HANDLERS[constants.SIGTERM] == [] def test_unregister_signal_not_found(self): - with self.assertRaises(LookupError): - base.unregister(base.signal.SIGTERM, 'test') + with pytest.raises(LookupError): + base.unregister(constants.SIGTERM, 'test') def test_unregister_handler_not_found(self): def handler(*args): return - base.register(base.signal.SIGTERM, handler) + base.register(constants.SIGTERM, handler) - with self.assertRaises(LookupError): - base.unregister(base.signal.SIGTERM, 'test') + with pytest.raises(LookupError): + base.unregister(constants.SIGTERM, 'test') From 14603bbc111b22676a20aaf071e22090bc24bac6 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:51:23 +0200 Subject: [PATCH 15/22] Fix tests.signals.test_process: replace os.waitpid() by os.wait() --- tests/signals/test_process.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/signals/test_process.py b/tests/signals/test_process.py index b8f4120..b3032b1 100644 --- a/tests/signals/test_process.py +++ b/tests/signals/test_process.py @@ -4,6 +4,15 @@ from pkit import signals from pkit.signals.base import call_signal_handler +from pkit.signals.base import SIGNAL_HANDLERS + + +def setup_module(): + pkit.signal.base.reset() + + +def teardown_module(): + pkit.signal.base.reset() def test_register_process(): @@ -20,7 +29,7 @@ def test_register_process(): process, process.on_sigchld) - with mock.patch('os.waitpid', return_value=(pid, 0)): + with mock.patch('os.wait', return_value=(pid, 0)): call_signal_handler(SIGCHLD)(SIGCHLD, None) assert vals == ['OK'] @@ -53,10 +62,12 @@ def test_register_two_process(): process2, process2.on_sigchld) - with mock.patch('os.waitpid', return_value=(pid1, 0)): + assert len(SIGNAL_HANDLERS[SIGCHLD]) == 1 + + with mock.patch('os.wait', return_value=(pid1, 0)): call_signal_handler(SIGCHLD)(SIGCHLD, None) - with mock.patch('os.waitpid', return_value=(pid2, 0)): + with mock.patch('os.wait', return_value=(pid2, 0)): call_signal_handler(SIGCHLD)(SIGCHLD, None) - assert vals == (1, 2) + assert vals == [1, 2] From 3e12585a40a571e37ce1e4d3fbbdf992f26fb8fc Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:53:07 +0200 Subject: [PATCH 16/22] Update test_pool: refactoring --- tests/test_pool.py | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/test_pool.py b/tests/test_pool.py index 4eb7af8..0555c6c 100644 --- a/tests/test_pool.py +++ b/tests/test_pool.py @@ -1,14 +1,30 @@ import pytest -import time import multiprocessing as mp from pkit.process import Process from pkit.pool import ProcessPool, Task from pkit.utils import wait +import pkit.signals -class TestTask: +def setup_module(): + pkit.signals.base.reset() + + +def teardown_module(): + pkit.signals.base.reset() + + +class PoolTestCase: + def setup_method(self): + pkit.signals.base.reset() + + def teardown_method(self): + pkit.signals.base.reset() + + +class TestTask(PoolTestCase): def test_task_has_default_status(self): t = Task(1234) assert t.status == Task.READY @@ -26,18 +42,22 @@ def test_finish_sets_status_to_finished(self): assert t.status == Task.FINISHED -class TestProcessPool: +class TestProcessPool(PoolTestCase): def test_execute_acquires_and_releases_slot(self): queue = mp.Queue() pp = ProcessPool(1) assert pp.slots.free == 1 - pp.execute(target=lambda q: q.get(), args=(queue,)) + pp.execute(target=queue.get) assert pp.slots.free == 0 - queue.put('abc') + queue.put('') - wait(until=lambda slots: slots.free == 1, args=(pp.slots,), timeout=0.5) + # If it timeouts, it means that pp.slots.release() was not called when + # the child process exited. + print('waiting...') + wait(until=lambda: pp.slots.free == 1, + timeout=0.5) assert pp.slots.free == 1 From 77dadc614a14dcbc5db2d00a1dcf750b12a755a4 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 22:53:37 +0200 Subject: [PATCH 17/22] Fix test_process: refactoring --- tests/test_process.py | 191 +++++++++++++++++------------------------- 1 file changed, 75 insertions(+), 116 deletions(-) diff --git a/tests/test_process.py b/tests/test_process.py index cae8498..b549786 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -1,4 +1,5 @@ import pytest +import multiprocessing import os import time @@ -7,82 +8,90 @@ import psutil from pkit.process import ProcessOpen, Process, get_current_process +import pkit.signals -def _collect_process(proc): - if proc.is_alive is True: - process_pid = proc.pid +def setup_module(): + pkit.signals.base.reset() - try: - proc.terminate(wait=True) - except OSError: - return - assert proc.is_alive is False +def teardown_module(): + pkit.signals.base.reset() - with pytest.raises(psutil.NoSuchProcess): - psutil.Process(process_pid).is_running() - try: - os.wait() - except OSError: - return +def collect_process(proc): + if proc.is_alive is False: + return + + process_pid = proc.pid + + try: + proc.terminate(wait=True) + except OSError: + return + + assert proc.is_alive is False + + with pytest.raises(psutil.NoSuchProcess): + psutil.Process(process_pid).is_running() + + try: + os.wait() + except OSError: + pass + + return -class TestGetCurrentProcess: +class ProcessTestCase(object): + def setup_method(self): + self.process = None + pkit.signals.base.reset() + + def teardown_method(self): + if self.process is not None: + collect_process(self.process) + self.process = None + + +class TestGetCurrentProcess(ProcessTestCase): def test_get_current_process_in_python_interpreter(self): current = get_current_process() assert isinstance(current, Process) is True assert current.pid == os.getpid() - assert current._child == None - assert current._parent == None + assert current._child is None + assert current._parent is None def test_get_current_process_while_process_runs(self): current = get_current_process() process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() - process_pid = process.pid assert hasattr(process, '_current') is True assert isinstance(process._current, Process) is True assert process._current != current assert process._current.pid != current.pid - _collect_process(process) - def test_get_current_process_is_reset_to_main_after_terminate(self): current = get_current_process() process = Process(target=lambda: time.sleep(0.1)) - + self.process = process process.start(wait=True) - process_pid = process.pid process.terminate(wait=True) assert hasattr(process, '_current') is True assert isinstance(process._current, Process) is True assert process._current.pid == current.pid - _collect_process(process) - - # def test_get_current_process_is_reset_to_main_after_join(self): - # current = get_current_process() - # process = Process(target=lambda: time.sleep(0.1)) - # process.start(wait=True) - # process_pid = process.pid - # process.join() - - # assert hasattr(process, '_current')) - # assert isinstance(process._current, Process)) - # assert process._current.pid == current.pid) - - -class TestProcessOpen: +class TestProcessOpen(ProcessTestCase): def test_init_with_wait_activated_actually_waits_for_process_to_be_ready(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process # default wait timeout lasts one second process_open = ProcessOpen(process, wait=True) @@ -93,20 +102,17 @@ def test_init_with_wait_activated_actually_waits_for_process_to_be_ready(self): # Ensure the ready flag has been awaited assert process_open.ready is True - - _collect_process(process) - def test_init_without_wait_activated_does_not_wait(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process_open = ProcessOpen(process) os.kill(process_open.pid, signal.SIGTERM) assert process_open.ready is False - _collect_process(process) - def test_init_with_wait_and_low_provided_wait_timeout(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process # Set up a really low wait timeout value to check if # wait is effectively too short for the ready flag to be @@ -114,10 +120,9 @@ def test_init_with_wait_and_low_provided_wait_timeout(self): process_open = ProcessOpen(process, wait=True, wait_timeout=0.000001) assert process_open.ready is False - _collect_process(process) - def test__send_ready_flag_closes_read_pipe_if_provided(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process read_pipe, write_pipe = os.pipe() process_open = ProcessOpen(process) @@ -126,10 +131,9 @@ def test__send_ready_flag_closes_read_pipe_if_provided(self): with pytest.raises(OSError): os.read(read_pipe, 128) - _collect_process(process) - def test__send_ready_flag_actually_sends_the_ready_flag(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process read_pipe, write_pipe = os.pipe() process_open = ProcessOpen(process) @@ -138,10 +142,9 @@ def test__send_ready_flag_actually_sends_the_ready_flag(self): read, _, _ = select.select([read_pipe], [], [], 0) assert len(read) == 1 - _collect_process(process) - def test__poll_ready_flag_closes_write_pipe_if_provided(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process read_pipe, write_pipe = os.pipe() process_open = ProcessOpen(process) @@ -150,10 +153,9 @@ def test__poll_ready_flag_closes_write_pipe_if_provided(self): with pytest.raises(OSError): os.write(write_pipe, str('abc 123').encode('UTF-8')) - _collect_process(process) - def test__poll_ready_flag_actually_recv_the_ready_flag(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process read_pipe, write_pipe = os.pipe() process_open = ProcessOpen(process) @@ -165,8 +167,6 @@ def test__poll_ready_flag_actually_recv_the_ready_flag(self): flag = process_open._poll_ready_flag(read_pipe) assert flag is True - _collect_process(process) - def test_non_blocking_poll_does_not_wait_for_process_end(self): short_target = lambda: time.sleep(0.1) @@ -240,6 +240,7 @@ def test_wait_with_timeout_shorter_than_execution_time_returns_none(self): def test_terminate_exits_with_failure_returncode(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process # Wait for the fork to be made, and the signal to be binded process_open = ProcessOpen(process, wait=True) @@ -249,8 +250,6 @@ def test_terminate_exits_with_failure_returncode(self): assert process_open.returncode == 1 - _collect_process(process) - def test_terminate_ignores_already_exited_processes(self): process_open = ProcessOpen(Process(target=None), wait=True) process_open.returncode = 24 @@ -262,15 +261,15 @@ def test_terminate_ignores_already_exited_processes(self): class TestProcess: def test__current_attribute_is_main_process_when_not_started(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process assert process._current is not None assert process._current.pid == os.getpid() assert process._current.name == 'MainProcess {0}'.format(process._current.pid) - _collect_process(process) - def test__current_attribute_is_process_when_started(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() pid_dump = process.pid @@ -284,10 +283,9 @@ def test__current_attribute_is_process_when_started(self): os.kill(pid_dump, signal.SIGTERM) process.wait() - _collect_process(process) - def test__current_attribute_is_main_process_when_stopped_with_terminate(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() pid_dump = process.pid @@ -305,29 +303,23 @@ def test__current_attribute_is_main_process_when_stopped_with_terminate(self): assert process._current.pid == os.getpid() assert process._current.name == 'MainProcess {0}'.format(process._current.pid) - _collect_process(process) - - def test__current_attribute_is_main_process_when_stopped_with_sigterm(self): - pass # See todo about sigterm proper support - def test_is_alive_is_false_when_in_parent_process(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process assert process.is_alive is False - _collect_process(process) - def test_is_alive_is_false_when_child_is_none(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process._child = None assert process.is_alive is False - _collect_process(process) - def test_is_alive_is_false_when_child_has_no_pid(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process child = ProcessOpen(process) child.pid = None @@ -335,10 +327,9 @@ def test_is_alive_is_false_when_child_has_no_pid(self): assert process.is_alive is False - _collect_process(process) - def test_is_alive_is_false_when_process_has_received_sigterm(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() pid_dump = process.pid @@ -351,10 +342,10 @@ def test_is_alive_is_false_when_process_has_received_sigterm(self): assert process.is_alive is False - _collect_process(process) - def test_is_alive_when_process_is_running(self): - process = Process(target=lambda: time.sleep(0.1)) + queue = multiprocessing.Queue() + process = Process(target=queue.get) + self.process = process process.start() pid_dump = process.pid @@ -368,30 +359,27 @@ def test_is_alive_when_process_is_running(self): os.kill(pid_dump, signal.SIGTERM) process.wait() - _collect_process(process) - def test_run_calls_target(self): def dummy_target(data): data['abc'] = '123' dummy_value = {'abc': None} p = Process(target=dummy_target, args=(dummy_value,)) + self.process = p p.run() assert 'abc' in dummy_value assert dummy_value, {'abc': '123'} - _collect_process(p) - def test_run_ignores_none_target(self): p = Process() + self.process = p p.run() - assert p.target == None - - _collect_process(p) + assert p.target is None def test_start_calls_run(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() pid_dump = process.pid @@ -408,10 +396,9 @@ def test_start_calls_run(self): with pytest.raises(psutil.NoSuchProcess): psutil.Process(pid_dump).is_running() - _collect_process(process) - def test_start_returns_process_pid(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process pid = process.start() pid_dump = process.pid @@ -424,10 +411,9 @@ def test_start_returns_process_pid(self): with pytest.raises(psutil.NoSuchProcess): psutil.Process(pid_dump).is_running() - _collect_process(process) - def test_start_raises_if_already_running(self): process = Process(target=lambda: time.sleep(0.1)) + self.process = process process.start() pid_dump = process.pid @@ -444,13 +430,12 @@ def test_start_raises_if_already_running(self): with pytest.raises(psutil.NoSuchProcess): psutil.Process(pid_dump).is_running() - _collect_process(process) - def test_join_awaits_on_process_exit(self): from multiprocessing import Queue queue = Queue() process = Process(target=lambda q: q.get(), args=(queue,)) + self.process = process process.start() pid_dump = process.pid @@ -460,21 +445,18 @@ def test_join_awaits_on_process_exit(self): queue.put('1') process.join() - _collect_process(process) - def test_join_raises_when_child_does_not_exist(self): process = Process() with pytest.raises(RuntimeError): process.join() - _collect_process(process) - def test_terminate_shutsdown_child_process(self): from multiprocessing import Queue queue = Queue() process = Process(target=lambda q: q.get(), args=(queue,)) + self.process = process process.start() pid_dump = process.pid @@ -483,40 +465,17 @@ def test_terminate_shutsdown_child_process(self): process.terminate(wait=True) - _collect_process(process) - - # def test_terminate_returns_a_failure_exit_code(self): - # process = Process(target=lambda: time.sleep(0.1)) - # process.start() - # pid_dump = process.pid - - # assert process.is_alive) - # assert psutil.Process(pid_dump).is_running()) - - # process.terminate(wait=True) - - # assert hasattr(process, '_exitcode')) - # assert process._exitcode >= 1) - - # assert process.is_alive) - # with pytest.raises(psutil.NoSuchProcess): - # psutil.Process(pid_dump).is_running() - def test_terminate_raises_when_child_does_not_exist(self): - process = Process() + self.process = Process() with pytest.raises(RuntimeError): - process.terminate() - - _collect_process(process) + self.process.terminate() def test_restart_raises_with_invalid_policy(self): - process = Process(target=lambda: time.sleep(0.1)) + self.process = Process(target=lambda: time.sleep(0.1)) with pytest.raises(ValueError): - process.restart("that's definetly invalid") - - _collect_process(process) + self.process.restart("that's definetly invalid") def test_process_on_exit_is_called(self): def acquire(): From 0db15e490777f65b8779d5c917e0b0983dc017bb Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 23:03:18 +0200 Subject: [PATCH 18/22] Update process: do not modify the Process instance from ProcessOpen --- pkit/process.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkit/process.py b/pkit/process.py index 3f1e8bc..a4944bc 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -159,8 +159,6 @@ def wait(self, timeout=None): delay = min(delay * 2, remaining, 0.05) time.sleep(delay) - self.process.clean() - return returncode def terminate(self): From 518554660ac0db7ed32db432ce57caa994f307e2 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 23:04:07 +0200 Subject: [PATCH 19/22] Update process: use signals.process to bind signals --- pkit/process.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkit/process.py b/pkit/process.py index a4944bc..47005a6 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -219,8 +219,9 @@ def __init__(self, target=None, name=None, self.target_kwargs = dict(kwargs) def bind_signal_handlers(self): - signal.signal(signal.SIGCHLD, self.on_sigchld) - signal.siginterrupt(signal.SIGCHLD, False) + signals.process.register( + signals.constants.SIGCHLD, + self, self.on_sigchld) def __str__(self): return '<{0} {1}>'.format(self.name, self.pid) @@ -229,6 +230,11 @@ def __repr__(self): return self.__str__() def on_sigchld(self, signum, sigframe): + signals.process.unregister( + signals.constants.SIGCHLD, + self, + self.on_sigchld) + if self._child is not None and self._child.pid: self.join() From 2dbb3fb11c1f9f4698129869ed1f8e95b236c6d4 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 23:07:08 +0200 Subject: [PATCH 20/22] Update process: add Process.exit() method --- pkit/process.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkit/process.py b/pkit/process.py index 47005a6..563aaa7 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -236,7 +236,7 @@ def on_sigchld(self, signum, sigframe): self.on_sigchld) if self._child is not None and self._child.pid: - self.join() + self.exit() def create(self): """Method to be called when the process child is forked""" @@ -293,13 +293,19 @@ def start(self, wait=False, wait_timeout=0): if self._child is not None: raise RuntimeError("Cannot start a process twice") - self.bind_signal_handlers() self._child = ProcessOpen(self, wait=wait, wait_timeout=wait_timeout) + self.bind_signal_handlers() child_pid = self._child.pid self._current = self return child_pid + def exit(self): + if self._on_exit: + self._on_exit(self) + + self.clean() + def join(self, timeout=None): """Awaits on Process exit @@ -311,15 +317,10 @@ def join(self, timeout=None): if self._child is None: raise RuntimeError("Can only join a started process") - try: - self._exitcode = self._child.wait(timeout) - except OSError: - pass - - if self._on_exit: - self._on_exit(self) + # FIXME: self._exitcode set inside self.wait() + self.wait() - self.clean() + self.exit() def terminate(self, wait=False): """Forces the process to stop @@ -334,7 +335,8 @@ def terminate(self, wait=False): self._child.terminate() if wait: - self.wait(until=lambda p, *args: p._child is None) + self.wait() + self.exit() def restart(self, policy=JOIN_RESTART_POLICY): if not policy in [JOIN_RESTART_POLICY, TERMINATE_RESTART_POLICY]: From 66472cd55ee8ca7569b7d966d027ac2036dd8a5d Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 23:07:47 +0200 Subject: [PATCH 21/22] Fix Process.wait() --- pkit/process.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pkit/process.py b/pkit/process.py index 563aaa7..b0ac1ca 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -386,12 +386,20 @@ def wait(self, until=None, args=(), timeout=None): :type timeout: float """ def default_until(self, *args): - if self._child is not None: - try: - self._child.wait(timeout) - except OSError: - pass - return True + if self._child is None: + return False + + exitcode = None + try: + # FIXME: I'm an ugly implicit side-effect! + exitcode = self._child.wait(timeout) + except OSError: + return False + + if exitcode is not None: + self._exitcode = exitcode + + return True if until is not None and not callable(until): raise ValueError("Until parameter must be a callable") From dd8d212a68b7c0673b572523d48948a5572e8f98 Mon Sep 17 00:00:00 2001 From: Greg Leclercq Date: Tue, 15 Apr 2014 23:08:13 +0200 Subject: [PATCH 22/22] Fix Process.is_alive --- pkit/process.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkit/process.py b/pkit/process.py index b0ac1ca..b7c96d9 100644 --- a/pkit/process.py +++ b/pkit/process.py @@ -417,9 +417,7 @@ def is_alive(self): if self._child is None or not self._child.pid: return False - self._child.poll() - - return self._child.returncode is None + return self._child.is_running @property def exitcode(self):