From ca4bece393f1ab1d8edbbc77bd8550b1e4ddb513 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 14 May 2015 15:36:51 -0700 Subject: [PATCH] Attempt to fix #378 --- .../plugins/standalone/authenticator.py | 19 ++++++++++++++++--- .../standalone/tests/authenticator_test.py | 16 ++++------------ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index 1558b86ca..7d5c3ea6e 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -152,9 +152,6 @@ class StandaloneAuthenticator(common.Plugin): :rtype: bool """ - signal.signal(signal.SIGIO, self.client_signal_handler) - signal.signal(signal.SIGUSR1, self.client_signal_handler) - signal.signal(signal.SIGUSR2, self.client_signal_handler) display = zope.component.getUtility(interfaces.IDisplay) @@ -259,6 +256,16 @@ class StandaloneAuthenticator(common.Plugin): :rtype: bool """ + # In order to avoid a race condition, we set the signal handler + # that will be needed by the parent process now, and undo this + # action if we turn out to be the child process. (This needs + # to happen before the fork because the child will send one of + # these signals to the parent almost immediately after the + # fork, and the parent must already be ready to receive it.) + signal.signal(signal.SIGIO, self.client_signal_handler) + signal.signal(signal.SIGUSR1, self.client_signal_handler) + signal.signal(signal.SIGUSR2, self.client_signal_handler) + fork_result = os.fork() Crypto.Random.atfork() if fork_result: @@ -269,6 +276,12 @@ class StandaloneAuthenticator(common.Plugin): return self.do_parent_process(port) else: # CHILD process (the TCP listener subprocess) + # Undo the parent's signal handler settings, which aren't + # applicable to us. + signal.signal(signal.SIGIO, signal.SIG_DFL) + signal.signal(signal.SIGUSR1, signal.SIG_DFL) + signal.signal(signal.SIGUSR2, signal.SIG_DFL) + self.child_pid = os.getpid() # do_child_process() is normally not expected to return but # should terminate via sys.exit(). diff --git a/letsencrypt/plugins/standalone/tests/authenticator_test.py b/letsencrypt/plugins/standalone/tests/authenticator_test.py index c6287774b..802431536 100644 --- a/letsencrypt/plugins/standalone/tests/authenticator_test.py +++ b/letsencrypt/plugins/standalone/tests/authenticator_test.py @@ -404,47 +404,39 @@ class DoParentProcessTest(unittest.TestCase): StandaloneAuthenticator self.authenticator = StandaloneAuthenticator(config=None, name=None) - @mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal") @mock.patch("letsencrypt.plugins.standalone.authenticator." "zope.component.getUtility") - def test_do_parent_process_ok(self, mock_get_utility, mock_signal): + def test_do_parent_process_ok(self, mock_get_utility): self.authenticator.subproc_state = "ready" result = self.authenticator.do_parent_process(1717) self.assertTrue(result) self.assertEqual(mock_get_utility.call_count, 1) - self.assertEqual(mock_signal.call_count, 3) - @mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal") @mock.patch("letsencrypt.plugins.standalone.authenticator." "zope.component.getUtility") - def test_do_parent_process_inuse(self, mock_get_utility, mock_signal): + def test_do_parent_process_inuse(self, mock_get_utility): self.authenticator.subproc_state = "inuse" result = self.authenticator.do_parent_process(1717) self.assertFalse(result) self.assertEqual(mock_get_utility.call_count, 1) - self.assertEqual(mock_signal.call_count, 3) - @mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal") @mock.patch("letsencrypt.plugins.standalone.authenticator." "zope.component.getUtility") - def test_do_parent_process_cantbind(self, mock_get_utility, mock_signal): + def test_do_parent_process_cantbind(self, mock_get_utility): self.authenticator.subproc_state = "cantbind" result = self.authenticator.do_parent_process(1717) self.assertFalse(result) self.assertEqual(mock_get_utility.call_count, 1) - self.assertEqual(mock_signal.call_count, 3) - @mock.patch("letsencrypt.plugins.standalone.authenticator.signal.signal") @mock.patch("letsencrypt.plugins.standalone.authenticator." "zope.component.getUtility") - def test_do_parent_process_timeout(self, mock_get_utility, mock_signal): + def test_do_parent_process_timeout(self, mock_get_utility): # Normally times out in 5 seconds and returns False. We can # now set delay_amount to a lower value so that it times out # faster than it would under normal use. result = self.authenticator.do_parent_process(1717, delay_amount=1) self.assertFalse(result) self.assertEqual(mock_get_utility.call_count, 1) - self.assertEqual(mock_signal.call_count, 3) class DoChildProcessTest(unittest.TestCase):